Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Medium] Denial of Service Pre-existing issues: - [High] Memory Leak - [High] Use-After-Free / Double Free -- commit 12d56e2f89cf46c2631f51c17242ae01f2acd00b Author: Lorenzo Stoakes (ARM) <[email protected]> mm/vma: enforce that only kernel-owned mappings may set VMA_IO_BIT This patch updates mmap_validate_vma_flags to warn and fail if a mapping sets VMA_IO_BIT without being marked as kernel-owned. > diff --git a/mm/vma.c b/mm/vma.c > index cb0c4c6257567..5996757d5aaf4 100644 > --- a/mm/vma.c > +++ b/mm/vma.c > @@ -2772,6 +2772,12 @@ static int mmap_validate_vma_flags(const vma_flags_t > *flags) > return -EINVAL; > #endif > > + if (!vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT)) { > + /* Only kernel-owned mappings may set VMA_IO_BIT. */ > + if (WARN_ON_ONCE(vma_flags_test(flags, VMA_IO_BIT))) [Severity: Medium] Can this warning be triggered by an unprivileged user intentionally mapping a device exposed by a buggy or out-of-tree driver? If a user interacts with a buggy driver that sets VMA_IO_BIT improperly, triggering this warning will crash the system if panic_on_warn=1 is configured. Is there a way to validate this without the warning to prevent a local denial of service? > + return -EINVAL; [Severity: High] This is a pre-existing issue, but does returning an error here expose a resource leak in call_mmap_prepare()? If __mmap_region() handles an mmap call for a driver that implements the .mmap_prepare hook, the driver may replace the mapping file: mm/vma.c:call_mmap_prepare() { ... err = mmap_prepare_validate(&prev_desc, desc); if (err) return err; /* Update fields permitted to be changed. */ map->pgoff = desc->pgoff; if (desc->vm_file != map->file) { ... map->file = desc->vm_file; map->file_doesnt_need_get = true; } ... } If mmap_prepare_validate() fails because of this new VMA_IO_BIT check returning an error, call_mmap_prepare() returns early. The state updates to map->file and map->file_doesnt_need_get are bypassed. When __mmap_region() aborts, it evaluates map.file_doesnt_need_get to decide whether to clean up the new file: mm/vma.c:__mmap_region() { ... if (map.file_doesnt_need_get) fput(map.file); ... } Since map.file_doesnt_need_get evaluates to false, is the reference to the newly allocated file permanently lost? [Severity: High] This is also a pre-existing issue, but can this failure path also lead to a double-free when mmap_file() fails? If a driver's .mmap_prepare hook successfully replaces the mapping file, call_mmap_prepare() sets map->file_doesnt_need_get = true. In __mmap_new_file_vma(), the get_file() call is skipped. If mmap_file() then fails (either because the driver's .mmap hook returns an error, or this validation fails), the error path unconditionally drops the reference: mm/vma.c:__mmap_new_file_vma() { ... error = mmap_file(map->file, vma, vmg); if (error) { ... fput(vma->vm_file); vma->vm_file = NULL; return error; } ... } The error propagates back to __mmap_region(), which executes the abort_munmap block: mm/vma.c:__mmap_region() { ... abort_munmap: ... if (map.file_doesnt_need_get) fput(map.file); ... } Because map.file_doesnt_need_get is still true, it calls fput() on the exact same file pointer, leading to a double-free. Does this new validation step make this sequence more reachable by unprivileged users interacting with out-of-tree drivers? > + } > + > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce...@kernel.org?part=24
