Re: [PATCH 3/3] mm: shmem: use mapping_set_frozen() for memfd preservation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: shmem: broken mapping_frozen() check ordering in shmem_setattr()
- [High] mm: shmem: shmem_write_begin() allows extending i_size on frozen mappings
--

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

mm: shmem: use mapping_set_frozen() for memfd preservation

This commit replaces the shmem-specific SHMEM_F_MAPPING_FROZEN flag with
generic page cache mapping_set_frozen() and mapping_frozen() helpers for
memfd live update preservation. It also updates shmem_setattr(),
shmem_write_begin(), and shmem_fallocate() to rely on the frozen mapping
state.

> diff --git a/mm/shmem.c b/mm/shmem.c
> index 6507b4246c8af..1c70fd306bec2 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1570,8 +1570,6 @@ static int shmem_setattr(struct mnt_idmap *idmap,
>  			return -EPERM;
>  
>  		if (newsize != oldsize) {
> -			if (info->flags & SHMEM_F_MAPPING_FROZEN)
> -				return -EPERM;
>  			error = shmem_reacct_size(SHMEM_I(inode)->flags,
>  					oldsize, newsize);
>  			if (error)
> @@ -1583,6 +1581,11 @@ static int shmem_setattr(struct mnt_idmap *idmap,
>  		}
>  		if (newsize <= oldsize) {
>  			loff_t holebegin = round_up(newsize, PAGE_SIZE);
> +
> +			/* No truncation allowed on a frozen mapping. */
> +			if (mapping_frozen(inode->i_mapping))
> +				return -EBUSY;

[Severity: High]
Does moving the frozen check after the newsize != oldsize block corrupt
inode->i_size and VM memory accounting on shrink, while allowing a frozen
memfd to grow via truncate()?

In mm/shmem.c:shmem_setattr(), when attr->ia_valid & ATTR_SIZE is set:

	if (newsize != oldsize) {
		error = shmem_reacct_size(SHMEM_I(inode)->flags,
				oldsize, newsize);
		if (error)
			return error;
		i_size_write(inode, newsize);
		update_mtime = true;
	}

When shrinking a frozen file (newsize < oldsize), shmem_reacct_size()
unaccounts VM_ACCT(oldsize) - VM_ACCT(newsize) via vm_unacct_memory() and
i_size_write() updates inode->i_size before the newsize <= oldsize block
checks mapping_frozen(inode->i_mapping) and returns -EBUSY without rolling
back either change or truncating the pages past the new EOF.

When growing a frozen file (newsize > oldsize), the newsize <= oldsize
branch is not taken at all, so mapping_frozen() is never checked and
inode->i_size increases even though memfd_luo_preserve() already recorded
ser->size and memfd_luo_freeze() only updates ser->pos.

Should mapping_frozen(inode->i_mapping) be checked before the
newsize != oldsize block so all ATTR_SIZE changes on frozen mappings are
rejected before modifying state?

> @@ -3482,10 +3485,6 @@ shmem_write_begin(const struct kiocb *iocb, struct address_space *mapping,
>  			return -EPERM;
>  	}
>  
> -	if (unlikely((info->flags & SHMEM_F_MAPPING_FROZEN) &&
> -		     pos + len > inode->i_size))
> -		return -EPERM;
> -
>  	ret = shmem_get_folio(inode, index, pos + len, &folio, SGP_WRITE);
>  	if (ret)
>  		return ret;

[Severity: High]
Can removing this check allow buffered writes to extend inode->i_size on a
frozen mapping when a folio is already resident at index?

If a preserved memfd has an unaligned i_size, a large folio spanning past
i_size, or folios preallocated beyond i_size with FALLOC_FL_KEEP_SIZE,
shmem_get_folio() finds the existing folio in i_pages without allocating and
inserting a new folio, so the frozen mapping check in page cache insertion
is not triggered.

Then mm/shmem.c:shmem_write_end() extends inode->i_size:

	if (pos + copied > inode->i_size)
		i_size_write(inode, pos + copied);

Because mm/memfd_luo.c:memfd_luo_preserve() captures ser->size during
preservation and memfd_luo_freeze() only updates ser->pos, any size extension
and data written past the preserved size will be lost across kexec.

Should shmem_write_begin() still reject writes with pos + len > inode->i_size
when mapping_frozen(mapping) is true?

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