Informational 24 Bitcoin Committed Aug 28, 2025 · 04:12 UTC

bkpr: don't leak event in fillin_apy_acct_details.

3e4c9221 View source ↗

Public commit record

What the developer wrote

Authored by Rusty Russell

73/100 · Adequate

bkpr: don't leak event in fillin_apy_acct_details.

Now accounts are not transient, we can't use them as temporary parents:

```
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: MEMLEAK: 0x5e3bf926ce28
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: label=plugins/bkpr/sql.c:109:struct chain_event
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: alloc:
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: ccan/ccan/tal/tal.c:488 (tal_alloc_)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/bkpr/sql.c:109 (chain_events)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/bkpr/sql.c:197 (chain_events_from_sql)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/bkpr/recorder.c:352 (find_chain_event_by_id)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/bkpr/channelsapy.c:120 (fillin_apy_acct_details)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/bkpr/channelsapy.c:172 (compute_channel_apys)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/bkpr/bookkeeper.c:220 (getblockheight_done)
lightningd-2 2025-08-28T02:36:35.629Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:1134 (handle_rpc_reply)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:1438 (rpc_read_response_one)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:1462 (rpc_conn_read_response)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/io.c:60 (next_plan)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/io.c:422 (do_plan)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/io.c:439 (io_ready)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/poll.c:455 (io_loop)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:2564 (plugin_main)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/bookkeeper.c:1547 (main)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ../sysdeps/nptl/libc_start_call_main.h:58 (__libc_start_call_main)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ../csu/libc-start.c:360 (__libc_start_main_impl)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: steal:
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/tal/tal.c:559 (tal_steal_)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/recorder.c:361 (find_chain_event_by_id)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/channelsapy.c:120 (fillin_apy_acct_details)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/channelsapy.c:172 (compute_channel_apys)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/bookkeeper.c:220 (getblockheight_done)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:1134 (handle_rpc_reply)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:1438 (rpc_read_response_one)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:1462 (rpc_conn_read_response)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/io.c:60 (next_plan)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/io.c:422 (do_plan)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/io.c:439 (io_ready)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ccan/ccan/io/poll.c:455 (io_loop)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/libplugin.c:2564 (plugin_main)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/bookkeeper.c:1547 (main)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ../sysdeps/nptl/libc_start_call_main.h:58 (__libc_start_call_main)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: ../csu/libc-start.c:360 (__libc_start_main_impl)
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: parents:
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/account.c:51:struct account
lightningd-2 2025-08-28T02:36:35.630Z **BROKEN** plugin-bookkeeper: plugins/bkpr/bookkeeper.c:1546:struct bkpr
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context

The short version

What changed, and why it matters

This is a one-line bug fix in Core Lightning's bookkeeper plugin. The developer changed a memory 'parent' pointer from a long-lived account object to a short-lived temporary context. Previously, a chain event object was being attached to an account that never gets freed, so the event leaked memory every time the code ran. The fix makes the event a temporary object that is cleaned up automatically. There is no attacker-controlled path; it is a routine memory leak in an internal accounting calculation.

Recommended action

Apply the patch. No urgent security response is needed; this is a reliability/fix-quality change. Operators running nodes with many closed channels may benefit from reduced memory growth over time.

Security signals we found

Memory leak (MEMLEAK) in bookkeeper plugin

Tal allocation parent changed from persistent account to temporary context

No input validation, boundary, or cryptographic change

Technical analysis

Evidence from the diff

In plugins/bkpr/channelsapy.c, fillin_apy_acct_details() calls find_chain_event_by_id() to look up a closing chain event. The old code passed acct (a persistent struct account) as the tal parent, but accounts are no longer transient, so the returned struct chain_event remained reachable through the account and was never freed. The patch changes the parent to tmpctx, the function’s temporary tal context, so the event is released when the function returns. The accompanying log shows a MEMLEAK report for struct chain_event with parents including struct account and struct bkpr, confirming the leak.

Changed components

plugins/bkpr/channelsapy.cplugins/bkpr/recorder.c (find_chain_event_by_id)bookkeeper plugin APY computation

Inspect captured patch +1 / −1

diff --git a/plugins/bkpr/channelsapy.c b/plugins/bkpr/channelsapy.c
index ed7f85b9..f4fa55a0 100644
--- a/plugins/bkpr/channelsapy.c
+++ b/plugins/bkpr/channelsapy.c
@@ -117,7 +117,7 @@ static void fillin_apy_acct_details(const struct bkpr *bkpr,
 
 	/* if this account is closed, add closing blockheight */
 	if (acct->closed_event_db_id) {
-		ev = find_chain_event_by_id(acct, bkpr, cmd,
+		ev = find_chain_event_by_id(tmpctx, bkpr, cmd,
 					    *acct->closed_event_db_id);
 		assert(ev);
 		apy->end_blockheight = ev->blockheight;

Risk score

Why this scored 24/100

Our methodology →

Potential impact 4/30

Exploitability 0/25

Stealth signal 3/15

Affected reach 3/15

Confidence 9/10

Evidence quality 5/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 24/100

Community notes

Related watches

Revert "doc: require Homebrew GNU make and gpatch on macOS"

Revert "tests: drop obsolete x-prefix in compacter-slow.sh"

doc: require Homebrew GNU make and gpatch on macOS