Re: [RFC PATCH 5/6] mm/memfd_luo: allow preserving a tmpfs file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] mm: restored file permission bits are corrupted by current_umask()
- [High] mm: TOCTOU race allows unprivileged userspace to disrupt tmpfs file retrieve
- [High] mm: memory leak of KHO folios on tmpfs_luo_file_retrieve failure
- [Medium] mm: potential double free / UAF of ser and folios on successful retrieve
--

commit 4087da995ba7fe7684120abaf9ec59deb366b97b
Author: Pratyush Yadav (Google) <pratyush@kernel.org>

mm/memfd_luo: allow preserving a tmpfs file

This commit introduces tmpfs file preservation across kexec live updates.
It preserves regular files within a tmpfs mount by serializing their
properties and memory folios, and restores them when the mount is
retrieved.

> diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
> --- a/mm/memfd_luo.c
> +++ b/mm/memfd_luo.c
> @@ -1005,62 +1005,51 @@ static int tmpfs_luo_file_retrieve(struct liveupdate_file_op_args *args)
[ ... ]
> 	err = liveupdate_get_file_incoming(args->session, ser->mnt_token,
> 					   &mnt_file);
> 	if (err) {
> 		pr_err("failed to retrieve tmpfs mount: %pe\n", ERR_PTR(err));
> 		goto free_ser;
> 	}

[Severity: High]
If this path jumps to the free_ser error label, does it permanently leak
the physical memory reserved for the KHO folios?

The free_ser error label frees the ser metadata, but does not discard
the physical folios or the ser->folios vmalloc region. Because the function
returns an error, tmpfs_luo_file_finish() may skip its cleanup logic if
args->retrieve_status is set to a non-zero error code. This would permanently
leak the memory of all files preserved inside that mount.

[ ... ]
> 	/*
> 	 * TODO: This is racy. This will link the file in the mount so after
> 	 * this call userspace can already open the file and write to it.
> 	 * Ideally we should first create the inode and set it up, and only
> 	 * then link it to the root.
> 	 */
> 	file = file_open_root(&mnt_file->f_path, ser->name,
> 			      O_RDWR | O_CREAT | O_EXCL | O_LARGEFILE,
> 			      ser->mode);

[Severity: High]
Does this code inadvertently strip the file permissions via the
orchestrator's umask?

VFS creation functions like file_open_root(O_CREAT) will apply
~current_umask() to the newly created file, which could alter the
preserved ser->mode. Since the kernel code lacks a subsequent chmod()
or vfs_setattr() step to enforce the exact preserved mode, the restored
file might have incorrect permission bits.

[Severity: High]
Does linking the file into the namespace here open a TOCTOU race window?

Since file_open_root() immediately links the restored file into the tmpfs
namespace, unprivileged userspace could potentially open and write or mmap
the file before memfd_luo_retrieve_folios() is called. This would instantiate
page cache folios, causing the subsequent shmem_add_to_page_cache() inside
memfd_luo_retrieve_folios() to hit an -EEXIST collision and permanently drop
the preserved data folios.

[ ... ]
> 	args->file = file;
> 	kho_restore_free(ser);
> 
> 	return 0;

[Severity: Medium]
Could this lead to a double-free or use-after-free regression of the KHO
folios on a successful retrieve?

This code frees the ser object but leaves args->serialized_data as a
dangling pointer.

[ ... ]
> static void tmpfs_luo_file_finish(struct liveupdate_file_op_args *args)
> {
> 	struct memfd_luo_folio_ser *folios_ser;
> 	struct tmpfs_luo_file_ser *ser;
> 
> 	/*
> 	 * A successful retrieve() already consumed the preserved memory, and a
> 	 * failed one cleaned up what it could. Only a file that was never
> 	 * retrieved is left to discard here.
> 	 */
> 	if (args->retrieve_status || !args->serialized_data)
> 		return;

[Severity: Medium]
If the LUO core framework sets args->retrieve_status to 0 upon a successful
return, tmpfs_luo_file_finish() might bypass this early return, dereference
the dangling args->serialized_data pointer, and mistakenly double-free the
folios. Could args->serialized_data be explicitly cleared after it is freed
to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923224408.3745689-1-pratyush@kernel.org?part=5