Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] File reference leak in error paths of `call_mmap_prepare()` and `__compat_vma_mmap()` when validation fails after file replacement. -- commit c2c06e31f69acde90d013dcbd918472d63f3996c Author: Lorenzo Stoakes (ARM) <[email protected]> mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma This commit adds validation to mmap_prepare_validate() to ensure that mmap actions are not set on VMAs that can be merged. It returns -EINVAL if such an action is detected. > diff --git a/mm/vma.c b/mm/vma.c > index a24f04428580b..9e45fc83f74c6 100644 > --- a/mm/vma.c > +++ b/mm/vma.c > @@ -2794,6 +2794,15 @@ static int mmap_validate(unsigned long prev_start, > int mmap_prepare_validate(const struct vm_area_desc *prev_desc, > const struct vm_area_desc *desc) > { > + /* > + * It is not valid to execute mmap actions for VMAs which can be merged, > + * as any such merge would leave portions of the mapping incorrectly > + * unmapped. > + */ > + if (vma_flags_can_merge(&desc->vma_flags) && > + WARN_ON_ONCE(desc->action.type != MMAP_NOTHING)) > + return -EINVAL; > + [Severity: High] This is a pre-existing issue, but by expanding the validation checks here, does this increase the surface area of a struct file leak in the error paths of call_mmap_prepare() and __compat_vma_mmap()? If a driver's mmap_prepare hook replaces the mapping's backing file, it returns a new file in desc->vm_file with an elevated refcount. If this newly added validation step fails, call_mmap_prepare() returns early: mm/vma.c:call_mmap_prepare() { ... err = mmap_prepare_validate(&prev_desc, desc); if (err) return err; This skips the crucial handoff that ensures the cleanup code in abort_munmap() will drop the reference to the new file: if (desc->vm_file != map->file) { map->file_doesnt_need_get = true; map->file = desc->vm_file; } ... } Since map->file and map->file_doesnt_need_get are not updated on error, the caller's cleanup code ignores the new file reference, leading to a permanent leak of the struct file. The same early return leak applies to __compat_vma_mmap() in mm/util.c, where desc is a stack variable and desc->vm_file is lost on error. Should these error paths handle fput() on desc->vm_file if it was replaced? > return mmap_validate(prev_desc->start, desc->start, > &prev_desc->vma_flags, &desc->vma_flags); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce...@kernel.org?part=4
