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

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) [off-list ref]
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.

[ ... ]

[ ... ]

+	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.

[ ... ]

[ ... ]

+	/* 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().

[ ... ]

[ ... ]

+	/* 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