linux-kernel - Re: [PATCH] mm/vma: use vmg->target to specify target VMA for new VMA merge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <85378739-d71e-4ccc-a46e-c6cc72915f04@lucifer.local>
Date: Thu, 19 Jun 2025 15:24:35 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
        Kees Cook <kees@...nel.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/vma: use vmg->target to specify target VMA for new
 VMA merge

On Thu, Jun 19, 2025 at 03:44:21PM +0200, Vlastimil Babka wrote:
> On 6/13/25 20:48, Lorenzo Stoakes wrote:
> > In commit 3a75ccba047b ("mm: simplify vma merge structure and expand
> > comments") we introduced the vmg->target field to make the merging of
> > existing VMAs simpler - clarifying precisely which VMA would eventually
> > become the merged VMA once the merge operation was complete.
> >
> > New VMA merging did not get quite the same treatment, retaining the rather
> > confusing convention of storing the target VMA in vmg->middle.
> >
> > This patch corrects this state of affairs, utilising vmg->target for this
> > purpose for both vma_merge_new_range() and also for vma_expand().
> >
> > We retain the WARN_ON for vmg->middle being specified in
> > vma_merge_new_range() as doing so would make no sense, but add an
> > additional debug assert for setting vmg->target.
> >
> > This patch additionally updates VMA userland testing to account for this
> > change.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@...e.cz>

Thanks!

>
> Nit below:
>
> > @@ -1086,27 +1087,29 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
> >   * @vmg: Describes a VMA expansion operation.
> >   *
> >   * Expand @vma to vmg->start and vmg->end.  Can expand off the start and end.
> > - * Will expand over vmg->next if it's different from vmg->middle and vmg->end ==
> > - * vmg->next->vm_end.  Checking if the vmg->middle can expand and merge with
> > + * Will expand over vmg->next if it's different from vmg->target and vmg->end ==
> > + * vmg->next->vm_end.  Checking if the vmg->target can expand and merge with
> >   * vmg->next needs to be handled by the caller.
> >   *
> >   * Returns: 0 on success.
> >   *
> >   * ASSUMPTIONS:
> > - * - The caller must hold a WRITE lock on vmg->middle->mm->mmap_lock.
> > - * - The caller must have set @vmg->middle and @vmg->next.
> > + * - The caller must hold a WRITE lock on vmg->target->mm->mmap_lock.
>
> The assert uses vmg->mm so maybe the comment should do the same? (IIRC mm
> was added only later to vmg?)

Haha you are the master of spotting stuff like this :)

Yeah you're right, this was actually updated for vma_merge_new_range() and
vma_merge_existing_range(), but missed this one.

Since we're updating it we may as well fix it, will send a fix-patch at
top-level :>)

>
> > + * - The caller must have set @vmg->target and @vmg->next.