From d467937c247fa739a4d535c792af467179db0228 Mon Sep 17 00:00:00 2001 From: Simon Vrouwe Date: Fri, 5 Nov 2021 10:39:08 +0200 Subject: [PATCH] fixup, restore freeing jsonrpc separately before ld, lesson learned Before this, below error happens: https://github.com/ElementsProject/lightning/runs/4097724359?check_suite_focus=true Valgrind error file: valgrind-errors.77564 ==77564== Invalid write of size 8 ==77564== at 0x1843C9: list_del_ (list.h:327) ==77564== by 0x1858E0: destroy_invoice_waiter (invoices.c:570) ==77564== by 0x220AC7: notify (tal.c:240) ==77564== by 0x220FDE: del_tree (tal.c:402) ==77564== by 0x221030: del_tree (tal.c:412) ==77564== by 0x221030: del_tree (tal.c:412) ==77564== by 0x221030: del_tree (tal.c:412) ==77564== by 0x22137A: tal_free (tal.c:486) ==77564== by 0x148212: main (lightningd.c:1214) ==77564== Address 0x63391b0 is 64 bytes inside a block of size 88 free'd ==77564== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==77564== by 0x2210AC: del_tree (tal.c:421) ==77564== by 0x221030: del_tree (tal.c:412) ==77564== by 0x221030: del_tree (tal.c:412) ==77564== by 0x22137A: tal_free (tal.c:486) ==77564== by 0x148212: main (lightningd.c:1214) ==77564== Block was alloc'd at ==77564== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==77564== by 0x220B35: allocate (tal.c:250) ==77564== by 0x2210F6: tal_alloc_ (tal.c:428) ==77564== by 0x184A7C: invoices_new (invoices.c:142) ==77564== by 0x187166: wallet_new (wallet.c:86) ==77564== by 0x147C77: main (lightningd.c:994) Invoice waiter is allocated off cmd, i.e. off ld->jsonrpc->commands but the list head wallet->invoices->waiters is allocated off wallet. So if jsonrpc happened to be freed after wallet, then the list head ptr is already gone and list_del doesn't know. There are many instances where list head is allocated off ld but element's (content) is allocated off the command. --- lightningd/jsonrpc.h | 2 +- lightningd/lightningd.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index 16fc01049fd7..b762ee48b186 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -22,7 +22,7 @@ enum command_mode { /* Context for a command (from JSON, but might outlive the connection!). */ /* FIXME: move definition into jsonrpc.c */ struct command { - /* Off json_cmd->commands */ + /* Off ld->jsonrpc->commands */ struct list_node list; /* The global state */ struct lightningd *ld; diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index fd905e46df8a..e98e539cafb5 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -1194,6 +1194,9 @@ int main(int argc, char *argv[]) /* Tell plugins we're shutting down, closes the db for write access. */ shutdown_plugins(ld); + /* Cleanup JSON RPC separately: destructors assume list pointers off ld */ + tal_free(ld->jsonrpc); + /* Clean our our HTLC maps, since they use malloc. */ htlc_in_map_clear(&ld->htlcs_in); htlc_out_map_clear(&ld->htlcs_out);