From 2c3543e42df841334affc446565d97e3ec540ce9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Apr 2020 19:49:46 +0930 Subject: [PATCH] lightningd: fix crash when failing htlc once channeld dies. We were reaching through it to get `ld`, but channeld is already dead. Caught this on test_onchaind_replay (without valgrind) on my test machine: INFO 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-chan#1: Peer transient failure in CHANNELD_NORMAL: channeld: Owning subdaemon channeld died (62208) **BROKEN** lightningd: FATAL SIGNAL 11 (version f6e1735) **BROKEN** lightningd: backtrace: common/daemon.c:44 (send_backtrace) 0x5634dc83dc55 **BROKEN** lightningd: backtrace: common/daemon.c:52 (crashdump) 0x5634dc83dca9 **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x7fd4b7c0b46f **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:285 (failmsg_incorrect_or_unknown) 0x5634dc82625a **BROKEN** lightningd: backtrace: lightningd/htlc_set.c:109 (htlc_set_add) 0x5634dc801e5c **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:496 (handle_localpay) 0x5634dc826997 **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:1008 (htlc_accepted_hook_callback) 0x5634dc827e60 **BROKEN** lightningd: backtrace: lightningd/plugin_hook.c:197 (plugin_hook_callback) 0x5634dc831ea1 **BROKEN** lightningd: backtrace: lightningd/plugin.c:261 (plugin_response_handle) 0x5634dc82d2c3 **BROKEN** lightningd: backtrace: lightningd/plugin.c:359 (plugin_read_json_one) 0x5634dc82d46f **BROKEN** lightningd: backtrace: lightningd/plugin.c:391 (plugin_read_json) 0x5634dc82d5c6 **BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:59 (next_plan) 0x5634dc896319 **BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:407 (do_plan) 0x5634dc896efe **BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:417 (io_ready) 0x5634dc896f40 Signed-off-by: Rusty Russell --- lightningd/htlc_set.c | 6 +++--- lightningd/invoice.c | 14 +++++++------- lightningd/peer_htlcs.c | 5 +++-- lightningd/peer_htlcs.h | 1 + lightningd/test/run-invoice-select-inchan.c | 1 + 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lightningd/htlc_set.c b/lightningd/htlc_set.c index 05bbd0089..ee27d90b8 100644 --- a/lightningd/htlc_set.c +++ b/lightningd/htlc_set.c @@ -106,7 +106,7 @@ void htlc_set_add(struct lightningd *ld, total_msat, payment_secret); if (!details) { local_fail_in_htlc(hin, - take(failmsg_incorrect_or_unknown(NULL, hin))); + take(failmsg_incorrect_or_unknown(NULL, ld, hin))); return; } @@ -130,7 +130,7 @@ void htlc_set_add(struct lightningd *ld, /* We check this now, since we want to fail with this as soon * as possible, to avoid other probing attacks. */ if (!payment_secret) { - local_fail_in_htlc(hin, take(failmsg_incorrect_or_unknown(NULL, hin))); + local_fail_in_htlc(hin, take(failmsg_incorrect_or_unknown(NULL, ld, hin))); return; } tal_arr_expand(&set->htlcs, hin); @@ -193,7 +193,7 @@ void htlc_set_add(struct lightningd *ld, /* This catches the case of the first payment in a set. */ if (!payment_secret) { htlc_set_fail(set, - take(failmsg_incorrect_or_unknown(NULL, hin))); + take(failmsg_incorrect_or_unknown(NULL, ld, hin))); return; } } diff --git a/lightningd/invoice.c b/lightningd/invoice.c index dcb4ca73b..cb4df790e 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -163,7 +163,7 @@ static void invoice_payload_remove_set(struct htlc_set *set, } static const u8 *hook_gives_failmsg(const tal_t *ctx, - struct log *log, + struct lightningd *ld, const struct htlc_in *hin, const char *buffer, const jsmntok_t *toks) @@ -181,7 +181,7 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx, if (json_tok_streq(buffer, resulttok, "continue")) { return NULL; } else if (json_tok_streq(buffer, resulttok, "reject")) { - return failmsg_incorrect_or_unknown(ctx, hin); + return failmsg_incorrect_or_unknown(ctx, ld, hin); } else fatal("Invalid invoice_payment hook result: %.*s", toks[0].end - toks[0].start, buffer); @@ -204,7 +204,7 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx, static bool warned = false; if (!warned) { warned = true; - log_unusual(log, + log_unusual(ld->log, "Plugin did not return object with " "'result' or 'failure_message' fields. " "This is now deprecated and you should " @@ -212,7 +212,7 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx, "{'result': 'reject'} or " "{'failure_message'... instead."); } - return failmsg_incorrect_or_unknown(ctx, hin); + return failmsg_incorrect_or_unknown(ctx, ld, hin); } if (!json_to_number(buffer, t, &val)) @@ -227,7 +227,7 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx, " changing to incorrect_or_unknown_payment_details", val); - return failmsg_incorrect_or_unknown(ctx, hin); + return failmsg_incorrect_or_unknown(ctx, ld, hin); } static void @@ -258,12 +258,12 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload, * we can also fail */ if (!wallet_invoice_find_by_label(ld->wallet, &invoice, payload->label)) { htlc_set_fail(payload->set, take(failmsg_incorrect_or_unknown( - NULL, payload->set->htlcs[0]))); + NULL, ld, payload->set->htlcs[0]))); return; } /* Did we have a hook result? */ - failmsg = hook_gives_failmsg(NULL, ld->log, + failmsg = hook_gives_failmsg(NULL, ld, payload->set->htlcs[0], buffer, toks); if (failmsg) { htlc_set_fail(payload->set, take(failmsg)); diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index d809ae15f..bfa8fe298 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -278,11 +278,12 @@ void local_fail_in_htlc_needs_update(struct htlc_in *hin, /* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */ const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx, + struct lightningd *ld, const struct htlc_in *hin) { return towire_incorrect_or_unknown_payment_details( ctx, hin->msat, - get_block_height(hin->key.channel->owner->ld->topology)); + get_block_height(ld->topology)); } /* localfail are for handing to the local payer if it's local. */ @@ -489,7 +490,7 @@ static void handle_localpay(struct htlc_in *hin, hin->cltv_expiry, get_block_height(ld->topology), ld->config.cltv_final); - failmsg = failmsg_incorrect_or_unknown(NULL, hin); + failmsg = failmsg_incorrect_or_unknown(NULL, ld, hin); goto fail; } diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index 02ed1c871..c24f76555 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -82,5 +82,6 @@ void json_format_forwarding_object(struct json_stream *response, const char *fie /* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */ const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx, + struct lightningd *ld, const struct htlc_in *hin); #endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */ diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index f0083949c..f1d998f7f 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -92,6 +92,7 @@ char *encode_scriptpubkey_to_addr(const tal_t *ctx UNNEEDED, { fprintf(stderr, "encode_scriptpubkey_to_addr called!\n"); abort(); } /* Generated stub for failmsg_incorrect_or_unknown */ const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx UNNEEDED, + struct lightningd *ld UNNEEDED, const struct htlc_in *hin UNNEEDED) { fprintf(stderr, "failmsg_incorrect_or_unknown called!\n"); abort(); } /* Generated stub for fatal */