Skip to content

Commit

Permalink
dual_funding: don't steal inflight field in update_channel_from_infli…
Browse files Browse the repository at this point in the history
…ght.

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 <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Jan 28, 2022
1 parent 1aa1e60 commit 58c9044
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,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;

Expand All @@ -1222,7 +1222,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(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;

Expand Down
2 changes: 1 addition & 1 deletion tests/test_closing.py
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,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()
Expand Down

0 comments on commit 58c9044

Please sign in to comment.