Skip to content

Commit

Permalink
fixup, restore freeing jsonrpc separately before ld, lesson learned
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
SimonVrouwe committed Nov 5, 2021
1 parent 8c85215 commit d467937
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lightningd/jsonrpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions lightningd/lightningd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d467937

Please sign in to comment.