Skip to content

Commit 4d1214b

Browse files
rustyrussellcdecker
authored andcommitted
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 <[email protected]>
1 parent 1be4d42 commit 4d1214b

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

lightningd/peer_control.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,11 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
957957
struct peer *peer = payload->peer;
958958
u8 *error;
959959

960+
/* Whatever happens, we free payload (it's currently a child
961+
* of the peer, which may be freed if we fail to start
962+
* subd). */
963+
tal_steal(tmpctx, payload);
964+
960965
/* If we had a hook, interpret result. */
961966
if (buffer) {
962967
const jsmntok_t *resulttok;
@@ -977,7 +982,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
977982
buffer + m->start);
978983
goto send_error;
979984
}
980-
tal_free(payload);
981985
return;
982986
} else if (!json_tok_streq(buffer, resulttok, "continue"))
983987
fatal("Plugin returned an invalid response to the connected "
@@ -1031,7 +1035,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
10311035
peer_restart_dualopend(peer, payload->pps,
10321036
channel, NULL);
10331037

1034-
tal_free(payload);
10351038
return;
10361039
#else
10371040
abort();
@@ -1044,7 +1047,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
10441047
channel->peer->addr = addr;
10451048
peer_start_channeld(channel, payload->pps,
10461049
NULL, true);
1047-
tal_free(payload);
10481050
return;
10491051

10501052
case CLOSINGD_SIGEXCHANGE:
@@ -1053,7 +1055,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
10531055
channel->peer->addr = addr;
10541056
peer_start_closingd(channel, payload->pps,
10551057
true, NULL);
1056-
tal_free(payload);
10571058
return;
10581059
}
10591060
abort();
@@ -1084,7 +1085,6 @@ peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
10841085
} else
10851086
#endif /* EXPERIMENTAL_FEATURES */
10861087
peer_start_openingd(peer, payload->pps, error);
1087-
tal_free(payload);
10881088
}
10891089

10901090
REGISTER_SINGLE_PLUGIN_HOOK(peer_connected,

0 commit comments

Comments
 (0)