Skip to content

common: explain why mlock() failed instead of a bare abort - #9438

Open
mattraydub wants to merge 1 commit into
ElementsProject:masterfrom
mattraydub:mlock-failure-diagnostic
Open

common: explain why mlock() failed instead of a bare abort#9438
mattraydub wants to merge 1 commit into
ElementsProject:masterfrom
mattraydub:mlock-failure-diagnostic

Conversation

@mattraydub

Copy link
Copy Markdown

mlock_tal_memory() aborts when sodium_mlock() fails, with no output at all:

void mlock_tal_memory(const tal_t *ptr)
{
	if (sodium_mlock((void *)ptr, tal_bytelen(ptr)) != 0)
		abort();
	tal_add_destructor(ptr, destroy_munlock);
}

There is no fallback path and no message, so the operator sees only:

lightning_hsmd: FATAL SIGNAL 6 (version v26.06.6)

How I found it

A node running fine on 25.09 died on every start after upgrading past 25.12.
Nothing in the logs pointed anywhere useful — lightningd: HSM sent unknown message type is emitted, but that is a red herring, just lightningd
misreading an already-dead subdaemon. It took a debug build to get the real
abort site:

0x825aa9918 abort+0x48
0x383372 mlock_tal_memory    common/utils.c:104
0x364b53 load_hsm            hsmd/hsmd.c:465
0x3641a7 init_hsm            hsmd/hsmd.c:556
0x363c17 handle_client       hsmd/hsmd.c:749

The cause was that FreeBSD jails deny mlock(2) unless the jail has
allow.mlock set. Granting it fixes the node with no other change. Because
25.09 and earlier did not lock the secret, the missing permission stays
invisible until the first upgrade past 25.12 — which makes it look like a
release regression rather than a configuration issue.

Containerized Linux deployments hit the same wall against RLIMIT_MEMLOCK
and fail exactly as opaquely.

Full write-up, including the confirmation that the stock package starts
normally once the privilege is granted:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=297253

What this changes

Only the diagnostic. The abort() stays deliberately — falling back to
unlocked memory would silently leave hsm_secret in swappable pages, and
that is a security policy change that shouldn't ride along on a patch about
error messages. If you'd rather have an opt-out flag for environments that
can't grant the privilege, I'm happy to follow up separately.

New output on failure:

FATAL: could not lock 32 bytes of sensitive memory into RAM: Operation not permitted
Memory locking is required to keep secrets out of swap.
If you are running in a container or jail, the privilege must be granted:
  FreeBSD jail: set allow.mlock=1 for the jail
  Linux: raise RLIMIT_MEMLOCK (ulimit -l), or grant the CAP_IPC_LOCK capability
  Docker/Podman: --ulimit memlock=-1:-1 or --cap-add=IPC_LOCK

Checks

  • tools/check-includes.sh passes (stdio.h / string.h added in the
    alphabetical position it expects)
  • format string builds clean under -Wformat=2 -Werror (%zu against
    tal_bytelen()'s size_t)

Andezion
Andezion previously approved these changes Aug 20, 2026

@Andezion Andezion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message only reaches stderr. Operators running lightningd under systemd typically only watch the cln log file, not raw stderr. Was routing this through status_failed/the log considered, or is matching the existing crashdump() convention intentional?

Comment thread common/utils.c

void mlock_tal_memory(const tal_t *ptr)
{
if (sodium_mlock((void *)ptr, tal_bytelen(ptr)) != 0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

daemon_setup() already calls err_set_progname(argv0) (via common_setup) before mlock_tal_memory() can run. Did you consider using ccan warn()/warnx() instead of a hand-rolled fprintf + strerror, so the message gets the daemon-name prefix for free and matches the warnx() call in crashdump() that immediately follows it on the same failure path?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — no, I'd missed that common_setup() calls err_set_progname(argv0)
(common/setup.c:43) before any of these call sites. Switched to warnx().

One wrinkle in case it comes up: warn() appends : strerror(errno) to the end of
the whole string, which would leave the errno dangling after the Docker/Podman line,
so I've kept warnx() with strerror(errno) inline on the first line where it
belongs. Dropping fprintf also lets the added #include <stdio.h> go again;
<string.h> stays for strerror.

Verified end-to-end on FreeBSD 15.1 / clang 19, in a jail with allow.mlock=0
(which is the case that prompted the patch):

lightning_hsmd: FATAL: could not lock 64 bytes of sensitive memory into RAM: Operation not permitted
Memory locking is required to keep secrets out of swap.
If you are running in a container or jail, the privilege must be granted:
  FreeBSD jail: set allow.mlock=1 for the jail
  Linux: raise RLIMIT_MEMLOCK (ulimit -l), or grant the CAP_IPC_LOCK capability
  Docker/Podman: --ulimit memlock=-1:-1 or --cap-add=IPC_LOCK
lightning_hsmd: FATAL SIGNAL 6 (version 1f86c30-modded)

Which I think makes your point better than I could: with warnx() the diagnostic and
the crashdump() line beneath it now carry the same lightning_hsmd: prefix and read
as one block. Full tree builds clean, no new warnings on common/utils.c (also checked
with -Werror -Wformat=2 added).

@Andezion
Andezion force-pushed the mlock-failure-diagnostic branch from 77fc67b to 1f86c30 Compare August 20, 2026 10:24
mlock_tal_memory() aborts when sodium_mlock() fails, with no output at
all.  The operator sees only "lightning_hsmd: FATAL SIGNAL 6" and has no
indication of the cause.

This is easy to hit.  FreeBSD jails deny mlock(2) unless the jail has
allow.mlock set, and containerized Linux deployments hit the same wall
against RLIMIT_MEMLOCK.  In both cases the failure is indistinguishable
from a crash.

Keep the abort -- falling back to unlocked memory would silently leave
the secret in swappable pages -- but say what failed and how to grant
the privilege.

Changelog-Fixed: Report the cause when locking secret memory fails, instead of aborting with no diagnostic.
@mattraydub

Copy link
Copy Markdown
Author

This message only reaches stderr. Operators running lightningd under systemd typically only watch the cln log file, not raw stderr. Was routing this through status_failed/the log considered, or is matching the existing crashdump() convention intentional?

Intentional, though I should have made that explicit in the commit message.

common/utils.c is linked into everything, not only subdaemons that have a status
fd. mlock_tal_memory() is also reached from common/hsm_secret.c (:66 and :445),
where status_setup_sync()/status_setup_async() has not necessarily run —
status_failed() funnels into status_send() on status_fd, so it isn't safe to
reach for from this layer.

Worth noting the log isn't entirely silent today either: the abort() raises
SIGABRT, crashdump() runs, and send_backtrace() forwards to lightningd via
bt_print, so on hsmd the failure does reach the cln log — just as a bare
FATAL SIGNAL 6 backtrace with no cause. This patch supplies the cause on the one
channel that exists at every call site, which is also what send_backtrace() itself
prefers: "We do stderr first, since it's most reliable."

Happy to revisit if you'd rather have a status-aware variant for the hsmd path
specifically, but that felt like more machinery than a one-shot startup diagnostic
warrants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants