lightningd: don't access after free on plugin crash

tests/test_plugin.py::test_important_plugin does this, and it's inelegant:

```
lightningd-1 2024-11-18T07:33:09.433Z **BROKEN** plugin-fail_by_itself.py: Plugin marked as important, shutting down lightningd!
lightningd-1 2024-11-18T07:33:09.451Z DEBUG   lightningd: io_break: lightningd_exit
lightningd-1 2024-11-18T07:33:09.533Z DEBUG   connectd: REPLY WIRE_CONNECTD_START_SHUTDOWN_REPLY with 0 fds
lightningd-1 2024-11-18T07:33:09.575Z DEBUG   lightningd: io_break: connectd_start_shutdown_reply
lightningd-1 2024-11-18T07:33:09.802Z DEBUG   lightningd: Looking for [autoclean,failedforwards,num]
{'github_repository': 'ElementsProject/lightning', 'github_sha': '0729de783e95c5208b1706f7d27b23904596bb71', 'github_ref': 'refs/pull/7835/merge', 'github_ref_name': 'HEAD', 'github_run_id': 11887300979, 'github_head_ref': 'guilt/fix-flakes8', 'github_run_number': 11566, 'github_base_ref': 'master', 'github_run_attempt': '1', 'testname': 'test_important_plugin', 'start_time': 1731915163, 'end_time': 1731915190, 'outcome': 'fail'}
----------------------------- Captured stderr call -----------------------------
No plugin for askrene-create-layer ?
Lost connection to the RPC socket.Reading JSON input: Connection reset by peerReading JSON input: Connection reset by peerReading JSON input: Connection reset by peerReading JSON input: Connection reset by peer
--------------------------- Captured stdout teardown ---------------------------
------------------------------- Valgrind errors --------------------------------
Valgrind error file: valgrind-errors.28639
==28639== Invalid read of size 8
==28639==    at 0x168310: command_exec (jsonrpc.c:808)
==28639==    by 0x168A98: rpc_command_hook_final (jsonrpc.c:954)
==28639==    by 0x1AD48C: plugin_hook_call_next (plugin_hook.c:196)
==28639==    by 0x1AD407: plugin_hook_callback (plugin_hook.c:183)
==28639==    by 0x1A6074: plugin_response_handle (plugin.c:663)
==28639==    by 0x1A62F0: plugin_read_json_one (plugin.c:775)
==28639==    by 0x1A652D: plugin_read_json (plugin.c:826)
==28639==    by 0x390200: next_plan (io.c:60)
==28639==    by 0x390E56: do_plan (io.c:422)
==28639==    by 0x390EBD: io_ready (io.c:439)
==28639==    by 0x3932F1: io_loop (poll.c:455)
==28639==    by 0x1ABBE4: shutdown_plugins (plugin.c:2588)
==28639==  Address 0x5d25a20 is 48 bytes inside a block of size 88 free'd
==28639==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==28639==    by 0x3A31FB: del_tree (tal.c:456)
==28639==    by 0x3A317B: del_tree (tal.c:447)
==28639==    by 0x3A34DC: tal_free (tal.c:532)
==28639==    by 0x1ABAF3: shutdown_plugins (plugin.c:2575)
==28639==    by 0x16E0D3: main (lightningd.c:1514)
==28639==  Block was alloc'd at
==28639==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==28639==    by 0x3A2BCA: allocate (tal.c:256)
==28639==    by 0x3A3252: tal_alloc_ (tal.c:473)
==28639==    by 0x1A815E: plugin_rpcmethod_add (plugin.c:1425)
==28639==    by 0x1A83F9: plugin_rpcmethods_add (plugin.c:1470)
==28639==    by 0x1A965A: plugin_parse_getmanifest_response (plugin.c:1850)
==28639==    by 0x1A971F: plugin_manifest_cb (plugin.c:1872)
==28639==    by 0x1A6074: plugin_response_handle (plugin.c:663)
==28639==    by 0x1A62F0: plugin_read_json_one (plugin.c:775)
==28639==    by 0x1A652D: plugin_read_json (plugin.c:826)
==28639==    by 0x390200: next_plan (io.c:60)
==28639==    by 0x390E56: do_plan (io.c:422)
==28639==
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2024-11-19 10:30:30 +10:30
parent 1a5916cb81
commit 1e8cbf2645

View File

@@ -2564,12 +2564,23 @@ void plugins_set_builtin_plugins_dir(struct plugins *plugins,
}
}
static bool release_request(const char *id,
struct jsonrpc_request *req,
struct plugin *plugin)
{
tal_del_destructor2(req, destroy_request, plugin);
return true;
}
void shutdown_plugins(struct lightningd *ld)
{
struct plugin *p, *next;
/* Tell them all to shutdown; if they care. */
list_for_each_safe(&ld->plugins->plugins, p, next, list) {
/* Clear all pending requests, so we don't try to answer them. */
strmap_iterate(&p->pending_requests, release_request, p);
strmap_clear(&p->pending_requests);
/* Kill immediately, deletes self from list. */
if (p->plugin_state != INIT_COMPLETE || !notify_plugin_shutdown(ld, p))
tal_free(p);