From 35ab9231636f3d232389270942e3293c8bc34bfd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 8 Mar 2016 10:36:15 +1030 Subject: [PATCH] peer: fix dangling peer->current_htlc->htlc pointer. It currently points into freed memory once we've make_commit_txs; we don't currently dereference it after that, but I did in some test code and got a surprise. Make a copy in all cases where we set it, so there can't be lifetime problems. Signed-off-by: Rusty Russell --- daemon/packets.c | 6 +++--- daemon/peer.c | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/daemon/packets.c b/daemon/packets.c index b47235c67..1e48599b4 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -513,7 +513,7 @@ Pkt *accept_pkt_htlc_routefail(const tal_t *ctx, goto fail; } - cur->htlc = &peer->cstate->a.htlcs[i]; + cur->htlc = tal_dup(cur, struct channel_htlc, &peer->cstate->a.htlcs[i]); /* Removing it should not fail: we regain HTLC amount */ cur->cstate = copy_funding(cur, peer->cstate); @@ -563,7 +563,7 @@ Pkt *accept_pkt_htlc_timedout(const tal_t *ctx, goto fail; } - cur->htlc = &peer->cstate->a.htlcs[i]; + cur->htlc = tal_dup(cur, struct channel_htlc, &peer->cstate->a.htlcs[i]); /* Do we agree it has timed out? */ if (controlled_time().ts.tv_sec < abs_locktime_to_seconds(&cur->htlc->expiry)) { @@ -619,7 +619,7 @@ Pkt *accept_pkt_htlc_fulfill(const tal_t *ctx, goto fail; } - cur->htlc = &peer->cstate->a.htlcs[i]; + cur->htlc = tal_dup(cur, struct channel_htlc, &peer->cstate->a.htlcs[i]); /* Removing it should not fail: they gain HTLC amount */ cur->cstate = copy_funding(cur, peer->cstate); diff --git a/daemon/peer.c b/daemon/peer.c index 54bec6ee8..f52401697 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -1288,7 +1288,8 @@ static void set_htlc_command(struct peer *peer, peer->current_htlc = tal(peer, struct htlc_progress); peer->current_htlc->cstate = tal_steal(peer->current_htlc, cstate); - peer->current_htlc->htlc = htlc; + peer->current_htlc->htlc = tal_dup(peer->current_htlc, + struct channel_htlc, htlc); if (r_fulfill) peer->current_htlc->r = *r_fulfill;