Re: [PATCH RFC 01/17] mm: shmem: Implement guest_memfd provider operations for tmpfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: guest_memfd bypasses tmpfs disk quotas and overcommit limits
- [High] mm: guest_memfd zone contamination via GFP_HIGHUSER_MOVABLE
- [Medium] fs: missing mnt_want_write() causes TOCTOU race with read-only remounts
--

commit aa746f432476f1dcdc507ac315f4952fd2b53724
Author: Ackerley Tng <ackerleytng@google.com>

mm: shmem: Implement guest_memfd provider operations for tmpfs

Implement guest_memfd provider operations to declare that tmpfs supports
providing memory to guest_memfd.

> diff --git a/mm/shmem.c b/mm/shmem.c
> index 897fa2b61346f..279f29a9861c1 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c

[ ... ]

> @@ -5234,6 +5235,114 @@ static const struct inode_operations shmem_special_inode_operations = {
>  #endif
>  };
>  
> +#ifdef CONFIG_KVM_GUEST_MEMFD
> +static void *shmem_gmem_attach(struct file *resource_file)
> +{

[ ... ]

> +	if (__mnt_is_readonly(resource_file->f_path.mnt))
> +		return ERR_PTR(-EROFS);

[Severity: Medium]
Does this check adequately protect against concurrent remounts?

shmem_gmem_attach() checks __mnt_is_readonly() without holding
mnt_want_write() or acquiring a VFS write reference. The mount could become
read-only immediately after this check, but guest_memfd would continue
allocating from it.

[ ... ]

> +static struct folio *shmem_gmem_alloc_folio(void *provider, pgoff_t index,
> +					    struct mempolicy *mpol)
> +{
> +	struct mempolicy *sb_mpol = NULL;
> +	struct vfsmount *mnt = provider;
> +	struct shmem_sb_info *sbinfo;
> +	struct folio *folio;
> +
> +	sbinfo = SHMEM_SB(mnt->mnt_sb);
> +	if (sbinfo->max_blocks &&
> +	    !percpu_counter_limited_add(&sbinfo->used_blocks,
> +					sbinfo->max_blocks, 1))
> +		return ERR_PTR(-ENOSPC);

[Severity: High]
Does this logic bypass tmpfs user/group disk quotas and VM overcommit limits?

While the global block limit is checked and charged here, it skips
both shmem_acct_blocks() (for VM overcommit) and dquot_alloc_block_nodirty()
(for inode disk quotas). Could an unprivileged user with write access to a
tmpfs mount evade limits and consume unbounded space?

> +
> +	if (!mpol) {
> +		sb_mpol = shmem_get_sbmpol(sbinfo);
> +		mpol = sb_mpol;
> +	}
> +
> +	if (mpol)
> +		folio = folio_alloc_mpol(GFP_HIGHUSER_MOVABLE, 0, mpol, index,
> +					 numa_node_id());
> +	else
> +		folio = folio_alloc(GFP_HIGHUSER_MOVABLE, 0);

[Severity: High]
Can placing unmovable guest_memfd pages into movable zones cause memory
fragmentation?

shmem_gmem_alloc_folio() allocates folios with the GFP_HIGHUSER_MOVABLE
flag. The __GFP_MOVABLE flag tells the allocator these pages can be migrated
or evicted, but guest_memfd pages are strictly unmovable. Does this break
memory management invariants and potentially cause large-order allocation or
memory hot-remove failures?

> +
> +	mpol_cond_put(sb_mpol);

[Severity: Medium]
This is a pre-existing issue, but does mpol_cond_put() leak the policy
reference count here?

shmem_get_sbmpol() returns a policy with an incremented refcount. However,
mpol_cond_put() only drops the reference if the MPOL_F_SHARED flag is set.
Since the superblock policy does not have MPOL_F_SHARED set, this operation
is a no-op, which appears to leak the policy reference.

> +
> +	if (!folio) {
> +		if (sbinfo->max_blocks)
> +			percpu_counter_sub(&sbinfo->used_blocks, 1);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	return folio;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-gmem-tmpfs-backend-v1-0-d36159822d18@google.com?part=1