On Thu, Sep 17, 2026 at 9:24 AM Lorenzo Stoakes (ARM) <[email protected]> wrote:
>
> It only makes sense to manipulate VMA fields if we allocated a new VMA,
> rather than merged it.
>
> VMA merging does not compare vm_ops or vm_private_data, so a merged VMA
> keeps its own, which is also what the legacy f_op->mmap path does since it
> never touches an existing VMA. Previously set_vma_user_defined_fields()
> overwrote the merged VMA's fields with those set for the new mapping. In
> practice these are the same values, with rare exceptions such as shmem
> selecting vm_ops based on whether the file has been unlinked, so no
> user-visible change is expected.
>
> Make this dependency explicit, and additionally constify have_mmap_prepare
> while we're here.

The fact that we might be overriding attributes of an existing VMA
that we merged with is technically a bug even if we never hit it,
right? If so, should we have:

Fixes: c84bf6dd2b83 ("mm: introduce new .mmap_prepare() file callback")

>
> Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>

Reviewed-by: Suren Baghdasaryan <[email protected]>

> ---
>  mm/vma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index fa784f069da4..4b74b48c29b7 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2894,7 +2894,7 @@ static unsigned long __mmap_region(struct file *file, 
> unsigned long addr,
>  {
>         struct mm_struct *mm = current->mm;
>         struct vm_area_struct *vma = NULL;
> -       bool have_mmap_prepare = file && file->f_op->mmap_prepare;
> +       const bool have_mmap_prepare = file && file->f_op->mmap_prepare;
>         VMA_ITERATOR(vmi, mm, addr);
>         const pgoff_t anon_pgoff = addr >> PAGE_SHIFT;
>         MMAP_STATE(map, mm, &vmi, addr, len, pgoff, anon_pgoff, vma_flags, 
> file);
> @@ -2937,7 +2937,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))
>                 set_vma_user_defined_fields(vma, &map);
>
>         __mmap_complete(&map, vma);
>
> --
> 2.55.0
>

Reply via email to