Skip to content

Commit 1e8cbf2

Browse files
committed
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 <[email protected]>
1 parent 1a5916c commit 1e8cbf2

File tree

1 file changed

+11
-0
lines changed

1 file changed

+11
-0
lines changed

lightningd/plugin.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2564,12 +2564,23 @@ void plugins_set_builtin_plugins_dir(struct plugins *plugins,
25642564
}
25652565
}
25662566

2567+
static bool release_request(const char *id,
2568+
struct jsonrpc_request *req,
2569+
struct plugin *plugin)
2570+
{
2571+
tal_del_destructor2(req, destroy_request, plugin);
2572+
return true;
2573+
}
2574+
25672575
void shutdown_plugins(struct lightningd *ld)
25682576
{
25692577
struct plugin *p, *next;
25702578

25712579
/* Tell them all to shutdown; if they care. */
25722580
list_for_each_safe(&ld->plugins->plugins, p, next, list) {
2581+
/* Clear all pending requests, so we don't try to answer them. */
2582+
strmap_iterate(&p->pending_requests, release_request, p);
2583+
strmap_clear(&p->pending_requests);
25732584
/* Kill immediately, deletes self from list. */
25742585
if (p->plugin_state != INIT_COMPLETE || !notify_plugin_shutdown(ld, p))
25752586
tal_free(p);

0 commit comments

Comments
 (0)