Re: [PATCH 15/39] mm/vma: only allow mmap to clear VMA_MAYWRITE_BIT if kernel-owned

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Leaked struct file reference when a driver replaces desc->vm_file but mmap_prepare_validate() subsequently fails.
--

commit bf1e56ed42b1d9fbc6eb9edeebea26ac54edd5c4
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/vma: only allow mmap to clear VMA_MAYWRITE_BIT if kernel-owned

This commit restricts the ability to clear the VMA_MAYWRITE_BIT to
kernel-owned mappings. It ensures that drivers which do not own the
mapping's contents cannot violate file system semantics by altering file
attributes. Any mmap hook attempting to do so without establishing a
kernel-owned mapping will now fail validation.

> diff --git a/mm/vma.c b/mm/vma.c
> index 9e45fc83f74c6..5264287532182 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2780,6 +2780,11 @@ static int mmap_validate(unsigned long prev_start,
>  	if (WARN_ON_ONCE(!was_maywrite && is_maywrite))
>  		return -EINVAL;
>  
> +	/* Only kernel-owned mappings may clear VMA_MAYWRITE_BIT. */
> +	if (!vma_flags_is_kernel_owned(curr_flags) &&
> +	    WARN_ON_ONCE(was_maywrite && !is_maywrite))
> +		return -EINVAL;
> +

[Severity: High]
This isn't a bug introduced by this patch, but this new validation failure
exposes a pre-existing resource leak. Does this code leak the struct file
reference if the driver legitimately replaced desc->vm_file during the
mmap_prepare hook?

If a driver's f_op->mmap_prepare hook swaps desc->vm_file (acquiring a new
file reference via split ownership) and then fails this new validation
step (e.g., clearing VMA_MAYWRITE_BIT without being kernel-owned),
call_mmap_prepare() will return early:

mm/vma.c:call_mmap_prepare() {
        ...
        err = mmap_prepare_validate(&prev_desc, desc);
        if (err)
                return err;
                
        map->pgoff = desc->pgoff;
        if (desc->vm_file != map->file) {
                map->file_doesnt_need_get = true;
                map->file = desc->vm_file;
        }
        ...
}

Because the early return happens before map->file and
map->file_doesnt_need_get are updated, the core cleanup path in
__mmap_region() remains unaware of the new file and fails to fput() it,
leaking the file reference and pinning the mount.

>  	return mmap_validate_vma_flags(curr_flags);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=15