From d9b1e6924376ef77733c9b3a1e4151cc5b193d52 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 30 Jan 2022 14:07:30 +1030 Subject: [PATCH] dual_funding: don't steal inflight field in update_channel_from_inflight. If we call update_channel_from_inflight *twice* with the same inflight, we will get bad results. Using tal_steal() here was a premature optimization: ``` Valgrind error file: valgrind-errors.496395 ==496395== Invalid read of size 8 ==496395== at 0x22A9D3: to_tal_hdr (tal.c:174) ==496395== by 0x22B4B5: tal_steal_ (tal.c:498) ==496395== by 0x16A13D: update_channel_from_inflight (peer_control.c:1225) ==496395== by 0x16A4C7: funding_depth_cb (peer_control.c:1299) ==496395== by 0x182807: txw_fire (watch.c:232) ==496395== by 0x182AA9: watch_topology_changed (watch.c:300) ==496395== by 0x1290ED: updates_complete (chaintopology.c:624) ==496395== by 0x129BF4: get_new_block (chaintopology.c:835) ==496395== by 0x125EEF: getrawblockbyheight_callback (bitcoind.c:362) ==496395== by 0x176ECC: plugin_response_handle (plugin.c:584) ==496395== by 0x1770F5: plugin_read_json_one (plugin.c:690) ==496395== by 0x1772D9: plugin_read_json (plugin.c:735) ==496395== Address 0x89fbb08 is 24 bytes inside a block of size 104 free'd ==496395== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==496395== by 0x22B193: del_tree (tal.c:421) ==496395== by 0x22B461: tal_free (tal.c:486) ==496395== by 0x16A123: update_channel_from_inflight (peer_control.c:1223) ==496395== by 0x16A4C7: funding_depth_cb (peer_control.c:1299) ==496395== by 0x182807: txw_fire (watch.c:232) ==496395== by 0x182AA9: watch_topology_changed (watch.c:300) ==496395== by 0x1290ED: updates_complete (chaintopology.c:624) ==496395== by 0x129BF4: get_new_block (chaintopology.c:835) ==496395== by 0x125EEF: getrawblockbyheight_callback (bitcoind.c:362) ==496395== by 0x176ECC: plugin_response_handle (plugin.c:584) ==496395== by 0x1770F5: plugin_read_json_one (plugin.c:690) ==496395== Block was alloc'd at ==496395== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==496395== by 0x22AC1C: allocate (tal.c:250) ==496395== by 0x22B1DD: tal_alloc_ (tal.c:428) ==496395== by 0x22B3A6: tal_alloc_arr_ (tal.c:471) ==496395== by 0x22C094: tal_dup_ (tal.c:805) ==496395== by 0x12B274: new_inflight (channel.c:187) ==496395== by 0x136D4C: wallet_commit_channel (dual_open_control.c:1260) ==496395== by 0x13B084: handle_commit_received (dual_open_control.c:2839) ==496395== by 0x13B6AF: dual_opend_msg (dual_open_control.c:2976) ==496395== by 0x1809FF: sd_msg_read (subd.c:553) ==496395== by 0x218F5D: next_plan (io.c:59) ==496395== by 0x219B65: do_plan (io.c:407) ``` Signed-off-by: Rusty Russell --- lightningd/peer_control.c | 4 ++-- tests/test_closing.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index ebcfcf4ed..7adec90f3 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1210,7 +1210,7 @@ static bool check_funding_tx(const struct bitcoin_tx *tx, static void update_channel_from_inflight(struct lightningd *ld, struct channel *channel, - struct channel_inflight *inflight) + const struct channel_inflight *inflight) { struct wally_psbt *psbt_copy; @@ -1223,7 +1223,7 @@ static void update_channel_from_inflight(struct lightningd *ld, channel->push = inflight->lease_fee; tal_free(channel->lease_commit_sig); channel->lease_commit_sig - = tal_steal(channel, inflight->lease_commit_sig); + = tal_dup_or_null(channel, secp256k1_ecdsa_signature, inflight->lease_commit_sig); channel->lease_chan_max_msat = inflight->lease_chan_max_msat; channel->lease_chan_max_ppt = inflight->lease_chan_max_ppt; diff --git a/tests/test_closing.py b/tests/test_closing.py index 481ac860a..61264648b 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -814,7 +814,7 @@ def test_channel_lease_falls_behind(node_factory, bitcoind): compact_lease=rates['compact_lease']) # sink the funding transaction - bitcoind.generate_block(1) + bitcoind.generate_block(1, wait_for_mempool=1) # stop l1 l1.stop()