lightningd: fix double-free when forking subdaemon fails.

payload is owned by the peer, which is freed in this case, then we
free payload (again).

==1404== Invalid read of size 8
==1404==    at 0x1F39E8: to_tal_hdr (tal.c:174)
==1404==    by 0x1F43A4: tal_free (tal.c:479)
==1404==    by 0x14B3D1: peer_connected_hook_cb (peer_control.c:1087)
==1404==    by 0x15D6E9: plugin_hook_call_ (plugin_hook.c:288)
==1404==    by 0x14B40E: plugin_hook_call_peer_connected (peer_control.c:1090)
==1404==    by 0x14B5B8: peer_connected (peer_control.c:1135)
==1404==    by 0x122FCF: connectd_msg (connect_control.c:310)
==1404==    by 0x160291: sd_msg_read (subd.c:480)
==1404==    by 0x15FBE7: read_fds (subd.c:308)
==1404==    by 0x1E37D1: next_plan (io.c:59)
==1404==    by 0x1E434E: do_plan (io.c:407)
==1404==    by 0x1E438C: io_ready (io.c:417)
==1404==  Address 0x2fcd2268 is 24 bytes inside a block of size 336 free'd
==1404==    at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1404==    by 0x1F416E: del_tree (tal.c:421)
==1404==    by 0x1F40F2: del_tree (tal.c:412)
==1404==    by 0x1F442C: tal_free (tal.c:486)
==1404==    by 0x148816: delete_peer (peer_control.c:120)
==1404==    by 0x148899: maybe_delete_peer (peer_control.c:136)
==1404==    by 0x13A970: destroy_uncommitted_channel (opening_common.c:29)
==1404==    by 0x1F3BB1: notify (tal.c:240)
==1404==    by 0x1F40A0: del_tree (tal.c:402)
==1404==    by 0x1F442C: tal_free (tal.c:486)
==1404==    by 0x13D3E9: peer_start_openingd (opening_control.c:911)
==1404==    by 0x14B3C2: peer_connected_hook_cb (peer_control.c:1086)
==1404==  Block was alloc'd at
==1404==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1404==    by 0x1F3C1B: allocate (tal.c:250)
==1404==    by 0x1F41B4: tal_alloc_ (tal.c:428)
==1404==    by 0x14B454: peer_connected (peer_control.c:1105)
==1404==    by 0x122FCF: connectd_msg (connect_control.c:310)
==1404==    by 0x160291: sd_msg_read (subd.c:480)
==1404==    by 0x15FBE7: read_fds (subd.c:308)
==1404==    by 0x1E37D1: next_plan (io.c:59)
==1404==    by 0x1E434E: do_plan (io.c:407)
==1404==    by 0x1E438C: io_ready (io.c:417)
==1404==    by 0x1E6552: io_loop (poll.c:445)
==1404==    by 0x12E2AD: io_loop_with_timers (io_loop_with_timers.c:24)

Fixes: #4329
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2021-01-14 10:36:50 +10:30 committed by Christian Decker
parent 1be4d42ca3
commit 4d1214b432

View File

@ -957,6 +957,11 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
struct peer *peer = payload->peer; struct peer *peer = payload->peer;
u8 *error; u8 *error;
/* Whatever happens, we free payload (it's currently a child
* of the peer, which may be freed if we fail to start
* subd). */
tal_steal(tmpctx, payload);
/* If we had a hook, interpret result. */ /* If we had a hook, interpret result. */
if (buffer) { if (buffer) {
const jsmntok_t *resulttok; const jsmntok_t *resulttok;
@ -977,7 +982,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
buffer + m->start); buffer + m->start);
goto send_error; goto send_error;
} }
tal_free(payload);
return; return;
} else if (!json_tok_streq(buffer, resulttok, "continue")) } else if (!json_tok_streq(buffer, resulttok, "continue"))
fatal("Plugin returned an invalid response to the connected " fatal("Plugin returned an invalid response to the connected "
@ -1031,7 +1035,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
peer_restart_dualopend(peer, payload->pps, peer_restart_dualopend(peer, payload->pps,
channel, NULL); channel, NULL);
tal_free(payload);
return; return;
#else #else
abort(); abort();
@ -1044,7 +1047,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
channel->peer->addr = addr; channel->peer->addr = addr;
peer_start_channeld(channel, payload->pps, peer_start_channeld(channel, payload->pps,
NULL, true); NULL, true);
tal_free(payload);
return; return;
case CLOSINGD_SIGEXCHANGE: case CLOSINGD_SIGEXCHANGE:
@ -1053,7 +1055,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
channel->peer->addr = addr; channel->peer->addr = addr;
peer_start_closingd(channel, payload->pps, peer_start_closingd(channel, payload->pps,
true, NULL); true, NULL);
tal_free(payload);
return; return;
} }
abort(); abort();
@ -1084,7 +1085,6 @@ send_error:
} else } else
#endif /* EXPERIMENTAL_FEATURES */ #endif /* EXPERIMENTAL_FEATURES */
peer_start_openingd(peer, payload->pps, error); peer_start_openingd(peer, payload->pps, error);
tal_free(payload);
} }
REGISTER_SINGLE_PLUGIN_HOOK(peer_connected, REGISTER_SINGLE_PLUGIN_HOOK(peer_connected,