Re: [PATCH v3 04/40] mm: consistently validate VMA state after mmap[_prepare] hooks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: UAF window caused by premature vma_close() before unmap_region()
- [Medium] mm: resource leak on mmap_prepare validation failure
- [Medium] mm: file reference leak when validation fails after mmap hook file swap
--

commit e5484b6aabe21d13a57f4db7fafb6d19bb0c0c97
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Subject: mm: consistently validate VMA state after mmap[_prepare] hooks

This commit centralises and extends validation of VMA state after executing
a driver's f_op->mmap or f_op->mmap_prepare hooks. It enforces restrictions
such as immutable VMA boundaries and prevents upgrading mappings to writable
if previously read-only.

> diff --git a/mm/internal.h b/mm/internal.h
> --- a/mm/internal.h
> +++ b/mm/internal.h
[ ... ]
> @@ -225,8 +243,12 @@ static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
>  {
[ ... ]
> +	err = mmap_hook_validate(prev_start, prev_end, &prev_flags, vma);
> +	if (unlikely(err)) {
> +		vma->vm_start = prev_start;
> +		vma->vm_end = prev_end;
> +		vma_close(vma);
>  	}

[Severity: High]
Could calling vma_close() here create a use-after-free window?

If a driver's f_op->mmap hook succeeds and populates PTEs (for example, via
remap_pfn_range()), but mmap_hook_validate() subsequently fails, the
driver's .close hook will free its backing resources immediately.

However, the PTEs are not cleared until __mmap_new_file_vma() later handles
the error:

mm/vma.c:__mmap_new_file_vma() {
    ...
    error = mmap_file(vma->vm_file, vma);
    map->vm_file = vma->vm_file;

    if (error) {
        ...
        /* Undo any partial mapping done by a device driver. */
        unmap_region(&unmap);
        return error;
    }
    ...
}

Does this leave the freed memory accessible to concurrent threads or lockless
GUP_fast walks through the stale PTEs until unmap_region() is reached?


[Severity: Medium]
Does this error path also leak the new file reference if a driver swaps the
file but then fails validation?

If a driver's mmap hook successfully replaces vma->vm_file (transferring a
new reference), mmap_file() updates map->vm_file. If mmap_hook_validate()
then fails, the error path in __mmap_new_file_vma() has this logic:

mm/vma.c:__mmap_new_file_vma() {
    ...
    if (error) {
        ...
        if (map_same_file(map))
            fput(map->vm_file);
            
        vma->vm_file = NULL;
        ...
    }
}

Because map->vm_file was updated to the newly swapped file, map_same_file()
will evaluate to false. This skips fput(map->vm_file) and clears
vma->vm_file to NULL, which appears to leak the new file reference.

> diff --git a/mm/util.c b/mm/util.c
> --- a/mm/util.c
> +++ b/mm/util.c
[ ... ]
> @@ -1224,19 +1224,28 @@ EXPORT_SYMBOL(compat_set_desc_from_vma);
>  int __compat_vma_mmap(struct vm_area_desc *desc,
>  		      struct vm_area_struct *vma)
>  {
[ ... ]
> +	/* Perform any preparatory tasks for mmap action. */
>  	err = mmap_action_prepare(desc);
> -	if (err) {
> -		if (desc->vm_file != vma->vm_file)
> -			fput(desc->vm_file);
> -		return err;
> -	}
> +	if (err)
> +		goto err_put;
> +	/* Check the caller did nothing crazy. */
> +	err = mmap_prepare_validate(&prev_desc, desc);
> +	if (err)
> +		goto err_put;

[Severity: Medium]
If mmap_prepare_validate() fails here, does this skip tearing down the
driver's prepared state?

The driver successfully executed its mmap_prepare hook prior to this check.
By jumping directly to err_put, the function returns an error without
invoking the descriptor's vm_ops->close hook, which might permanently leak
any resources or hardware state allocated during vfs_mmap_prepare().

> diff --git a/mm/vma.c b/mm/vma.c
> --- a/mm/vma.c
> +++ b/mm/vma.c
[ ... ]
> @@ -2862,6 +2882,11 @@ static int call_mmap_prepare(struct mmap_state *map,
>  		struct vm_area_desc *desc)
>  {
[ ... ]
> +	/* Check the caller did nothing crazy. */
> +	err = mmap_prepare_validate(&prev_desc, desc);
> +	if (err)
> +		return err;

[Severity: Medium]
Similar to the compatibility layer above, does returning directly on
validation failure leak the resources allocated by vfs_mmap_prepare()?

Without calling the driver's close hook, it seems the driver has no way to
clean up its established state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-b4-mmap-prepare-vma-flag-sanify-v3-0-4583d8a23bca@kernel.org?part=4