vincenzopalazzo
/
rgb-cln
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

lightningd: fix use-after-free during shutdown. 

When we are calling hooks, we track them via a linked list.  As they
execute, we pop them off the list in plugin_hook_killed().

When we kill a plugin, we have a destructor which remove its entry from the linked list: plugin_hook_killed.

If it's at the head of the list, that means the plugin died while
processing the hook, so instead of just deleting it, we call
plugin_hook_killed() which behaves as if it said "result: continue".

But plugin_hook_killed() just returns if we're shutting down; this
leaves the link (then freed) on the list, and the *next* plugin tries
to unlink from the list, accessing the previous free entry.

The fix is simple: unlink from the list in plugin_hook_killed() even
if we're shutting down.

```
Valgrind error file: valgrind-errors.78570
==78570== Invalid write of size 8
==78570==    at 0x174B55: list_del_ (list.h:328)
==78570==    by 0x174FCC: plugin_hook_killed (plugin_hook.c:135)
==78570==    by 0x21DC3F: notify (tal.c:240)
==78570==    by 0x21E156: del_tree (tal.c:402)
==78570==    by 0x21E1A8: del_tree (tal.c:412)
==78570==    by 0x21E4F2: tal_free (tal.c:486)
==78570==    by 0x16EBD1: plugin_kill (plugin.c:345)
==78570==    by 0x16F9C4: plugin_conn_finish (plugin.c:724)
==78570==    by 0x20F1A5: destroy_conn (poll.c:244)
==78570==    by 0x20F1C9: destroy_conn_close_fd (poll.c:250)
==78570==    by 0x21DC3F: notify (tal.c:240)
==78570==    by 0x21E156: del_tree (tal.c:402)
==78570==  Address 0x6aee688 is 40 bytes inside a block of size 72 free'd
==78570==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==78570==    by 0x21E224: del_tree (tal.c:421)
==78570==    by 0x21E1A8: del_tree (tal.c:412)
==78570==    by 0x21E4F2: tal_free (tal.c:486)
==78570==    by 0x16EBD1: plugin_kill (plugin.c:345)
==78570==    by 0x16F9C4: plugin_conn_finish (plugin.c:724)
==78570==    by 0x20F1A5: destroy_conn (poll.c:244)
==78570==    by 0x20F1C9: destroy_conn_close_fd (poll.c:250)
==78570==    by 0x21DC3F: notify (tal.c:240)
==78570==    by 0x21E156: del_tree (tal.c:402)
==78570==    by 0x21E4F2: tal_free (tal.c:486)
==78570==    by 0x20D7B6: io_close (io.c:450)
==78570==  Block was alloc'd at
==78570==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==78570==    by 0x21DCAD: allocate (tal.c:250)
==78570==    by 0x21E26E: tal_alloc_ (tal.c:428)
==78570==    by 0x175599: plugin_hook_call_ (plugin_hook.c:259)
==78570==    by 0x13616F: plugin_hook_call_onion_message_blinded (onion_message.c:126)
==78570==    by 0x13643B: handle_obs_onionmsg_to_us (onion_message.c:187)
==78570==    by 0x138BBD: gossip_msg (gossip_control.c:140)
==78570==    by 0x178AEC: sd_msg_read (subd.c:495)
==78570==    by 0x20CA00: next_plan (io.c:59)
==78570==    by 0x20D608: do_plan (io.c:407)
==78570==    by 0x20D64A: io_ready (io.c:417)
==78570==    by 0x20F8F1: io_loop (poll.c:445)
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2021-10-01 06:49:32 +09:30 committed by Christian Decker
parent d54594c3dc
commit 89d143bc63
1 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lightningd/plugin_hook.c
View File

@ -157,19 +157,20 @@ static void plugin_hook_callback(const char *buffer, const jsmntok_t *toks,
struct plugin_hook_call_link *last, *it;
bool in_transaction = false;
log_debug(r->ld->log, "Plugin %s returned from %s hook call",
r->plugin->shortname, r->hook->name);
/* Pop the head off the call chain and continue with the next */
last = list_pop(&r->call_chain, struct plugin_hook_call_link, list);
assert(last != NULL);
tal_del_destructor(last, plugin_hook_killed);
tal_free(last);
if (r->ld->state == LD_STATE_SHUTDOWN) {
log_debug(r->ld->log,
"Abandoning plugin hook call due to shutdown");
return;
}
/* Pop the head off the call chain and continue with the next */
last = list_pop(&r->call_chain, struct plugin_hook_call_link, list);
assert(last != NULL);
tal_del_destructor(last, plugin_hook_killed);
tal_free(last);
log_debug(r->ld->log, "Plugin %s returned from %s hook call",
r->plugin->shortname, r->hook->name);
if (buffer) {
resulttok = json_get_member(buffer, toks, "result");