From 7604f27fb8751b4c5f833a53706e2408efdefb8a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 23 Apr 2018 19:38:01 +0930 Subject: [PATCH] lightningd: make sure openingd and uncommitted_channel free each other. Without this, we can get errors on shutdown: Valgrind error file: valgrind-errors.27444 ==27444== Invalid read of size 8 ==27444== at 0x1950E2: secp256k1_pubkey_load (secp256k1.c:127) ==27444== by 0x19CF87: secp256k1_ec_pubkey_serialize (secp256k1.c:189) ==27444== by 0x14FED9: towire_pubkey (towire.c:59) ==27444== by 0x15AAFB: towire_gossipctl_peer_disconnected (gen_gossip_wire.c:969) ==27444== by 0x1253EF: opening_channel_errmsg (opening_control.c:526) ==27444== by 0x1386A3: destroy_subd (subd.c:589) ==27444== by 0x18222C: notify (tal.c:240) ==27444== by 0x1826E1: del_tree (tal.c:400) ==27444== by 0x182733: del_tree (tal.c:410) ==27444== by 0x182733: del_tree (tal.c:410) ==27444== by 0x182B1F: tal_free (tal.c:511) ==27444== by 0x11FC53: main (lightningd.c:410) ==27444== Address 0x6c3af98 is 72 bytes inside a block of size 216 free'd ==27444== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==27444== by 0x1827BC: del_tree (tal.c:421) ==27444== by 0x182B1F: tal_free (tal.c:511) ==27444== by 0x11F3C7: shutdown_subdaemons (lightningd.c:211) ==27444== by 0x11FC27: main (lightningd.c:406) ==27444== Block was alloc'd at ==27444== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==27444== by 0x182296: allocate (tal.c:250) ==27444== by 0x182863: tal_alloc_ (tal.c:448) ==27444== by 0x12F2DF: new_peer (peer_control.c:74) ==27444== by 0x125600: new_uncommitted_channel (opening_control.c:576) ==27444== by 0x125870: peer_accept_channel (opening_control.c:668) ==27444== by 0x13032A: peer_sent_nongossip (peer_control.c:427) ==27444== by 0x116B9E: peer_nongossip (gossip_control.c:60) ==27444== by 0x116F2B: gossip_msg (gossip_control.c:172) ==27444== by 0x138323: sd_msg_read (subd.c:503) ==27444== by 0x137C02: read_fds (subd.c:330) ==27444== by 0x175550: next_plan (io.c:59) Signed-off-by: Rusty Russell --- lightningd/opening_control.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 62ff9f92c..1381e3b77 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -115,6 +115,7 @@ void kill_uncommitted_channel(struct uncommitted_channel *uc, /* Close openingd. */ subd_release_channel(uc->openingd, uc); + uc->openingd = NULL; if (uc->fc) command_fail(uc->fc->cmd, "%s", why); @@ -407,6 +408,8 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, command_success(fc->cmd, response); subd_release_channel(openingd, fc->uc); + fc->uc->openingd = NULL; + /* Frees fc too, and tmpctx */ tal_free(fc->uc); return; @@ -415,6 +418,7 @@ failed: close(fds[0]); close(fds[1]); subd_release_channel(openingd, fc->uc); + fc->uc->openingd = NULL; /* Frees fc too, and tmpctx */ tal_free(fc->uc); } @@ -496,6 +500,7 @@ static void opening_fundee_finished(struct subd *openingd, fds[0], fds[1], funding_signed, false); subd_release_channel(openingd, uc); + uc->openingd = NULL; tal_free(uc); } @@ -536,6 +541,12 @@ static void opening_channel_set_billboard(struct uncommitted_channel *uc, static void destroy_uncommitted_channel(struct uncommitted_channel *uc) { + if (uc->openingd) { + struct subd *openingd = uc->openingd; + uc->openingd = NULL; + subd_release_channel(openingd, uc); + } + uc->peer->uncommitted_channel = NULL; /* Last one out frees */