[PATCH] mm/memfd_luo: carry the exec-bit state in the preserved record

[PATCH] mm/memfd_luo: carry the exec-bit state in the preserved record

[Haolong Wang]

F_SEAL_EXEC freezes the exec bits as they are at sealing time: a memfd
becomes either permanently executable or permanently un-executable.
The preserved state of a memfd (struct memfd_luo_ser) carries the
seals but not the inode mode, and the restore path creates the file
with memfd_alloc_file("", MFD_ALLOW_SEALING) -- the default
(executable) mode -- without consulting the vm.memfd_noexec policy
(that policy is applied on the memfd_create() syscall path only).

A memfd sealed as un-executable is therefore restored as mode 0777
with F_SEAL_EXEC re-applied on top of it: the guarantee inverts.
Because the restored file is executable, memfd_add_seals() also
implies F_SEAL_WRITE, F_SEAL_FUTURE_WRITE, F_SEAL_SHRINK and
F_SEAL_GROW (the W^X rule), so the restored memfd comes back with its
write seals locked and its exec bits permanently irrevocable
(shmem_setattr() rejects any exec-bit change).  This is a functional
regression that needs no attacker: the owner's own pwrite() to the
memfd succeeds before the update and fails with EPERM after it.  The
inversion is also silent: F_GET_SEALS keeps reporting the seal.

Carry the fact in the existing ser->flags field, which commit
8a552d68a86e ("mm: memfd_luo: preserve file seals") added for exactly
this kind of extension ("Unused flag bits must be set to 0"): preserve
records whether the inode had exec bits, and retrieve reproduces them
by setting the mode of the fresh anonymous inode directly, before the
seals are re-applied -- the W^X rule in memfd_add_seals() looks at the
mode when it decides whether to imply the write seals, so the mode has
to be in place first.  The file is not published at that point, so no
other thread can observe the intermediate state.  Reusing the
MFD_NOEXEC_SEAL alloc flag was considered and dropped: it would
silently add F_SEAL_EXEC to memfds that never had it (a plain
non-executable memfd without the seal would come back with the seal
attached and its exec bits irrevocable), and it would couple the
restore to the current policy rather than to the state that was
preserved.  A record that does not say the bits were set is restored
without them -- which is what a memfd created under vm.memfd_noexec=2
looks like, and is also the safe reading for records written before
this bit existed.

With this, the round trip is exact: a NX memfd (0666 + 0x20) is
restored as 0666 + 0x20 and stays writable by its owner; a plain
non-executable memfd (0666, unsealed) stays 0666 and unsealed, its
exec bits still chmod-able; the baseline executable memfd stays 0777
and still execve's; and memfd_create() under vm.memfd_noexec=2 still
forces NX on fresh memfds.  No seal is invented for any of them.

Refusing to preserve memfds with F_SEAL_EXEC was considered and
rejected: with vm.memfd_noexec >= 1 every memfd created without an
explicit MFD_EXEC gets MFD_NOEXEC_SEAL forced on it (and = 2 refuses
the MFD_EXEC opt-out outright), so the refusal would disable the whole
preservation feature exactly on the systems that harden the most.
Preserving the full inode mode would be the alternative, but one bit in
the already-extensible flags field is sufficient and keeps the record
layout unchanged.

One point for the maintainers: a clear exec bit in a record is
ambiguous between "written before this bit existed" and "the bits were
clear", and both readings are safe -- the memfd comes back
non-executable, which is what a memfd created under vm.memfd_noexec=2
looks like.  For an old record it does change one case: an executable
memfd with F_SEAL_EXEC preserved by a kernel without this change is
restored as permanently non-executable instead of permanently
executable.  If that is not acceptable, the reading has to be tied to a
version bump instead: commit 8a552d68a86e ("mm: memfd_luo: preserve
file seals") already noted that new semantics may require one ("they
will need another version bump").

Measured on a QEMU live-update run (preserve -> kexec -> retrieve):
before the fix the restored NX memfd is mode 0777, seals 0x3e, execve
of its contents succeeds and the owner cannot write to it; with the
fix the same test reports the round trip exact (mode 0666, seals 0x20,
still writable, execve -> EACCES), a plain non-executable memfd comes
back 0666 with no seal and can be chmod'ed executable again, and the
baseline executable memfd and fresh memfds are unaffected.

Fixes: 8a552d68a86e ("mm: memfd_luo: preserve file seals")
Assisted-by: Pi:glm-5.3-flash
Signed-off-by: Haolong Wang <18766350203@163.com>
---
 include/linux/kho/abi/memfd.h |  3 +++
 mm/memfd_luo.c                | 31 +++++++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/linux/kho/abi/memfd.h b/include/linux/kho/abi/memfd.h
index 08b10fea2afc..471cfe8a4616 100644
--- a/include/linux/kho/abi/memfd.h
+++ b/include/linux/kho/abi/memfd.h
@@ -87,6 +87,9 @@ struct memfd_luo_ser {
 	struct kho_vmalloc folios;
 } __packed;
 
+/* The execute bits were set on the memfd when its seals were applied. */
+#define MEMFD_LUO_FL_EXEC_BIT		BIT(0)
+
 /* The compatibility string for memfd file handler */
 #define MEMFD_LUO_FH_COMPATIBLE	"memfd-v2"
 
diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
index 59de210bee5f..950a43a4c328 100644
--- a/mm/memfd_luo.c
+++ b/mm/memfd_luo.c
@@ -300,6 +300,17 @@ static int memfd_luo_preserve(struct liveupdate_file_op_args *args)
 	ser->size = inode_size;
 	ser->seals = seals;
 
+	/*
+	 * F_SEAL_EXEC freezes the exec bits as they are at sealing time, so
+	 * the restore has to reproduce them.  Record whether the inode had
+	 * exec bits here; the restore sets the mode of the fresh inode
+	 * accordingly before re-applying the seals.  ser is zero-initialized,
+	 * so a clear bit means the bits were clear (or the record predates
+	 * this flag -- the safe reading for those is also "no exec bits").
+	 */
+	if (inode->i_mode & 0111)
+		ser->flags |= MEMFD_LUO_FL_EXEC_BIT;
+
 	err = memfd_luo_preserve_folios(args->file, &ser->folios,
 					&folios_ser, &nr_folios);
 	if (err)
@@ -533,8 +544,11 @@ static int memfd_luo_retrieve(struct liveupdate_file_op_args *args)
 	}
 
 	/*
-	 * The seals are preserved. Allow sealing here so they can be added
-	 * later.
+	 * F_SEAL_EXEC freezes the execute bits as they were when the seal was
+	 * applied, so the restore has to reproduce them.  A record that does
+	 * not say the bits were set is restored without them, which is what a
+	 * memfd created under vm.memfd_noexec=2 looks like and is also the safe
+	 * reading for records written before this flag existed.
 	 */
 	file = memfd_alloc_file("", MFD_ALLOW_SEALING);
 	if (IS_ERR(file)) {
@@ -543,6 +557,19 @@ static int memfd_luo_retrieve(struct liveupdate_file_op_args *args)
 		goto free_ser;
 	}
 
+	/*
+	 * Reproduce the exec bits before the seals are re-applied: F_SEAL_EXEC
+	 * freezes the bits as they were at sealing time, and the W^X rule in
+	 * memfd_add_seals() looks at the mode when it decides whether to imply
+	 * the write seals.  Setting the mode here, directly on the fresh
+	 * anonymous inode, is safe -- the file has not been published and no
+	 * other thread can reach it yet.
+	 */
+	if (ser->flags & MEMFD_LUO_FL_EXEC_BIT)
+		file->f_inode->i_mode |= 0111;
+	else
+		file->f_inode->i_mode &= ~0111;
+
 	err = memfd_add_seals(file, ser->seals);
 	if (err) {
 		pr_err("failed to add seals: %pe\n", ERR_PTR(err));
-- 
2.56.0

Re: [PATCH] mm/memfd_luo: carry the exec-bit state in the preserved record

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: TOCTOU race in memfd_luo_preserve loses fchmod and seals updates
- [Medium] mm/memfd_luo: leak of preserved folios in memfd_luo_retrieve() error paths
--

commit b09cba94dd95aadc45f383b05eb9e6f205bf88e7
Author: Haolong Wang <18766350203@163.com>
Subject: mm/memfd_luo: carry the exec-bit state in the preserved record

This commit preserves the execution bits of a memfd during a KHO live
update. It records the exec-bit state in the memfd_luo_ser structure and
applies it to the fresh inode during restore before re-applying seals,
ensuring the W^X policy behaves correctly.

> diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
> --- a/mm/memfd_luo.c
> +++ b/mm/memfd_luo.c
> @@ -299,6 +299,17 @@ static int memfd_luo_preserve(struct liveupdate_file_op_args *args)
>  	ser->size = inode_size;
>  	ser->seals = seals;
>  
> +	/*
> +	 * F_SEAL_EXEC freezes the exec bits as they are at sealing time, so
> +	 * the restore has to reproduce them.  Record whether the inode had
> +	 * exec bits here; the restore sets the mode of the fresh inode
> +	 * accordingly before re-applying the seals.  ser is zero-initialized,
> +	 * so a clear bit means the bits were clear (or the record predates
> +	 * this flag -- the safe reading for those is also "no exec bits").
> +	 */
> +	if (inode->i_mode & 0111)
> +		ser->flags |= MEMFD_LUO_FL_EXEC_BIT;
> +

[Severity: High]
Can a concurrent userspace modification race with this early caching?

The i_mode is read and cached here, before dropping the inode_lock. Shortly
after, shmem_freeze() is called. However, SHMEM_F_MAPPING_FROZEN ignores
ATTR_MODE in shmem_setattr() and is not checked in memfd_add_seals().

Because memfd_luo_freeze() blindly updates only f_pos without re-evaluating
the mode or seals:

mm/memfd_luo.c:memfd_luo_freeze() {
    ...
    ser->pos = args->file->f_pos;
    ...
}

Could userspace modify the mode via fchmod or add seals between the preserve
and freeze phases, causing the restore process to lose those updates and
break security policies like the W^X rule?

[ ... ]

> @@ -542,6 +556,19 @@ static int memfd_luo_retrieve(struct liveupdate_file_op_args *args)
>  		goto free_ser;
>  	}
>  
> +	/*
> +	 * Reproduce the exec bits before the seals are re-applied: F_SEAL_EXEC
> +	 * freezes the bits as they were at sealing time, and the W^X rule in
> +	 * memfd_add_seals() looks at the mode when it decides whether to imply
> +	 * the write seals.  Setting the mode here, directly on the fresh
> +	 * anonymous inode, is safe -- the file has not been published and no
> +	 * other thread can reach it yet.
> +	 */
> +	if (ser->flags & MEMFD_LUO_FL_EXEC_BIT)
> +		file->f_inode->i_mode |= 0111;
> +	else
> +		file->f_inode->i_mode &= ~0111;
> +
>  	err = memfd_add_seals(file, ser->seals);
>  	if (err) {
>  		pr_err("failed to add seals: %pe\n", ERR_PTR(err));
>  		goto put_file;
>  	}

[Severity: Medium]
Do these error paths leak the physical folios tracked by ser->folios?

When jumping to free_ser or put_file on failure, kho_restore_free(ser) is
called, but there is no call to restore the vmalloc metadata via
kho_restore_vmalloc() and free the physical folios.

Looking at memfd_luo_finish(), the driver explicitly expects retrieve() to
clean up its own folios on failure:

mm/memfd_luo.c:memfd_luo_finish() {
    /*
     * If retrieve was successful, nothing to do. If it failed, retrieve()
     * already cleaned up everything it could. So nothing to do there
     * either. Only need to clean up when retrieve was not called.
     */
    if (args->retrieve_status)
        return;
    ...
}

Will failing at memfd_alloc_file() or memfd_add_seals() leave these
KHO-preserved folios permanently reserved in memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014003558.357783-1-18766350203@163.com?part=1

Re: [PATCH] mm/memfd_luo: carry the exec-bit state in the preserved record

[Haolong Wang]

Thanks for the review.  Both findings were real.

On the exec-mode snapshot: the sample itself was atomic -- the mode is
read under inode_lock, in the same critical section that sets
SHMEM_F_MAPPING_FROZEN -- but it was taken far too early.  The frozen
mapping only rejects growing, shrinking and fallocate(); fchmod() of the
exec bits (until F_SEAL_EXEC is set) and F_ADD_SEALS are not checked on
that path at all, and freeze() refreshed only f_pos.  v2 closes this from
both ends: freeze() takes the final sample of the exec mask and the seals
under the inode lock and, in the same critical section, sets a new
SHMEM_F_SEAL_STATE_FROZEN inode flag before dropping the lock.  From
that point on shmem_setattr() rejects exec-bit changes, memfd_add_seals()
rejects new seals, and shmem_set_acl() rejects access-ACL updates.  The
flag is cleared by a new .unfreeze callback when the update is rolled
back, and defensively by unpreserve().

Two more things fell out of that work:

 - A single "some exec bit was set" flag was not enough: an execute mask
   of 0100 (mode 0700) came back as 0111, which is the inversion this
   series is about.  The record now carries the three execute bits
   separately, retrieve() restores the exact mask and rejects unknown flag
   bits, and luo_kexec_simple.c round-trips an execute mask of 0100 with
   its full 0x3e seal set, plus a memfd without execute bits
   (MFD_NOEXEC_SEAL: mask 0000, seal set 0x20, still writable), which is
   the state the original report was about.
 - An access ACL rewrites inode->i_mode without going through
   shmem_setattr(), so the frozen state has to cover that path too.
   Scope note: F_SEAL_EXEC itself is still bypassable by an access ACL
   write (posix_acl_update_mode() updates the mode directly, with no seal
   check anywhere on that path).  That is a pre-existing
   F_SEAL_EXEC/vm.memfd_noexec issue, deliberately not part of this
   series; I will report it separately with a minimal repro.

On the folio leak: also real.  retrieve() now releases the preserved
folios on every failure path by sharing the cleanup with
memfd_luo_finish().  The folio array is restored before the file is
allocated, so the later error paths (memfd_alloc_file(),
memfd_add_seals()) can release the folios directly instead of allocating
memory while already handling a failure, and memfd_luo_retrieve_folios()
takes ownership as soon as it runs, so nothing is released twice.  It
predates the exec-bit work, so it is a separate commit with a Fixes tag.

Two limitations I could not close here, both written down in the patches:

 - f_pos can still change after freeze().  The LUO file API has no global
   userspace quiescence before .freeze() runs, so this needs a
   framework-level answer rather than a memfd one.
 - If kho_restore_vmalloc() fails before consuming anything (for example
   its own kvmalloc() fails), the folio array cannot be walked at all and
   those folios stay reserved.  That API is not failure-atomic and cannot
   be retried after a partial failure, so it needs a KHO-side fix or an
   allocation-free discard/iterator API.

On record versions: the record is now written as "memfd-v3", and a
read-only "memfd-v2" handler is registered next to it.  The compatible
strings are matched exactly, so the matrix is:

  record writer   reading kernel and handler     result
  old v2          old kernel, v2 handler         default executable mode
  old v2          new kernel, legacy v2 reader   no execute bits
  new v3          new kernel, v3 handler         exact exec mask and seals
  new v3          old kernel                     no handler, -ENOENT

Rows two and four are both behaviour changes, and neither direction is
interoperable: an executable memfd preserved by an older kernel comes back
without execute bits (row two -- the safe reading; the old reader never
looked at the flags, it restored the default executable mode), and a new
record handed to an older kernel finds no matching handler at all, so the
deserialization fails with -ENOENT instead of the memfd being restored as
executable again (row four, which is why the version is bumped).  The
legacy handler only reads records written by an older kernel: its
.can_preserve() always returns false, so an outgoing preservation is
always serialized as "memfd-v3".

v2 will follow as a separate thread.

Both changes are covered by a QEMU live-update run (preserve -> kexec ->
retrieve) with the liveupdate selftest: the memfd with execute mask 0100
and seal set 0x3e round-trips exactly, the memfd without execute bits
comes back with mask 0000, seal set 0x20 and still writable, and the same
test against a kernel without the change reports "execute mask changed
across kexec: 0111 != 0100".  The numbers are in the v2 cover letter.