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) <[email protected]> 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-4583d8a23...@kernel.org?part=4
