Re: [PATCH 01/39] mm/vma: predicate setting mmap_prepare VMA fields on new vma alloc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] File reference leak when a file is swapped during mmap_prepare and the mapping is subsequently merged.
- [High] File reference leak in call_mmap_prepare() error paths when validation fails after file replacement.
--

commit 8df6960e843adc0f8e640f756781ceaed05911b5
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/vma: predicate setting mmap_prepare VMA fields on new vma alloc

This commit ensures that VMA fields are only manipulated if a new VMA is
allocated, rather than merged, as VMA merging preserves existing fields.
It also constifies have_mmap_prepare.

> diff --git a/mm/vma.c b/mm/vma.c
> index 97567fb7ef33d..2a90c509bb312 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
[ ... ]
> @@ -2909,7 +2909,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  		allocated_new = true;
>  	}
>  
> -	if (have_mmap_prepare && !map_is_anon(&map))
> +	if (have_mmap_prepare && allocated_new && !map_is_anon(&map))

[Severity: High]
This is a pre-existing issue, but does __mmap_region() leak a file reference
if the mapping is successfully merged after the file is swapped?

If the VMA is successfully merged with an adjacent mapping:

mm/vma.c:__mmap_region() {
    ...
    if (map.prev || map.next) {
        VMG_MMAP_STATE(vmg, &map, /* vma = */ NULL);
        vma = vma_merge_new_range(&vmg);
    }
    ...
}

allocated_new remains false, bypassing __mmap_new_vma() and the normal
consumption of the swapped file reference.

The function then returns on success without executing an fput(map.file) to
clean up the unconsumed extra reference when map.file_doesnt_need_get is
true and allocated_new is false.

[Severity: High]
This is also a pre-existing issue, but does call_mmap_prepare() leak a file
reference in its error paths when validation fails after a file replacement?

If vfs_mmap_prepare() successfully replaces desc->vm_file, but a subsequent
validation check fails:

mm/vma.c:call_mmap_prepare() {
    ...
    err = vfs_mmap_prepare(map->file, desc);
    if (err)
        return err;

    if (!desc->vm_ops)
        return -EINVAL;

    err = call_action_prepare(map, desc);
    if (err)
        return err;
    ...
}

The function returns early without updating map->file_doesnt_need_get. This
leaves the newly acquired desc->vm_file unreferenced by the error teardown
logic in __mmap_region(), leading to a leak.

>  		set_vma_user_defined_fields(vma, &map);
>  
>  	__mmap_complete(&map, vma);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=1