linux-kernel - Re: [PATCH 4/4] mm: abstract VMA extension and merge into 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] [day] [month] [year] [list]

Message-ID: <ba75d32d-521a-4c8d-8b4c-fa491131e262@lucifer.local>
Date:   Mon, 9 Oct 2023 19:22:07 +0100
From:   Lorenzo Stoakes <lstoakes@...il.com>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        "=Liam R . Howlett" <Liam.Howlett@...cle.com>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 4/4] mm: abstract VMA extension and merge into
 vma_merge_extend() helper

On Mon, Oct 09, 2023 at 06:30:02PM +0200, Vlastimil Babka wrote:
> On 10/8/23 22:23, Lorenzo Stoakes wrote:
> > mremap uses vma_merge() in the case where a VMA needs to be extended. This
> > can be significantly simplified and abstracted.
> >
> > This makes it far easier to understand what the actual function is doing,
> > avoids future mistakes in use of the confusing vma_merge() function and
> > importantly allows us to make future changes to how vma_merge() is
> > implemented by knowing explicitly which merge cases each invocation uses.
> >
> > Note that in the mremap() extend case, we perform this merge only when
> > old_len == vma->vm_end - addr. The extension_start, i.e. the start of the
> > extended portion of the VMA is equal to addr + old_len, i.e. vma->vm_end.
> >
> > With this refactoring, vma_merge() is no longer required anywhere except
> > mm/mmap.c, so mark it static.
> >
> > Signed-off-by: Lorenzo Stoakes <lstoakes@...il.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@...e.cz>

Thanks!

>
> Nit:
> > @@ -2546,6 +2546,24 @@ static struct vm_area_struct *vma_merge_new_vma(struct vma_iterator *vmi,
> >  			 vma->vm_userfaultfd_ctx, anon_vma_name(vma));
> >  }
> >
> > +/*
> > + * Expand vma by delta bytes, potentially merging with an immediately adjacent
> > + * VMA with identical properties.
> > + */
> > +struct vm_area_struct *vma_merge_extend(struct vma_iterator *vmi,
> > +					struct vm_area_struct *vma,
> > +					unsigned long delta)
> > +{
> > +	pgoff_t pgoff = vma->vm_pgoff +
> > +		((vma->vm_end - vma->vm_start) >> PAGE_SHIFT);
>
> could use vma_pages() here

Will update in v2.

>
> > +
> > +	/* vma is specified as prev, so case 1 or 2 will apply. */
> > +	return vma_merge(vmi, vma->vm_mm, vma, vma->vm_end, vma->vm_end + delta,
> > +			 vma->vm_flags, vma->anon_vma, vma->vm_file, pgoff,
> > +			 vma_policy(vma), vma->vm_userfaultfd_ctx,
> > +			 anon_vma_name(vma));
> > +}
> > +
> >  /*
> >   * do_vmi_align_munmap() - munmap the aligned region from @start to @end.
> >   * @vmi: The vma iterator