Re: [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

-cc my personal mail

Hi Jakub,

I realise this is from an old report so totally forgivable :P but please
use my work mail, everything that goes to my personal mail from the mailing
lists is >/dev/null :)

I only noticed this because I was checking for something else ;)

The patch looks fine to me btw!

Cheers, Lorenzo

On Wed, Jun 04, 2025 at 12:38:30PM +0000, Jakub Acs wrote:
> commit 270aa010620697fb27b8f892cc4e194bc2b7d134 upstream.
>
> Patch series "mm/uffd: Fix vma merge/split", v2.
>
> This series contains two patches that fix vma merge/split for userfaultfd
> on two separate issues.
>
> Patch 1 fixes a regression since 6.1+ due to something we overlooked when
> converting to maple tree apis.  The plan is we use patch 1 to replace the
> commit "2f628010799e (mm: userfaultfd: avoid passing an invalid range to
> vma_merge())" in mm-hostfixes-unstable tree if possible, so as to bring
> uffd vma operations back aligned with the rest code again.
>
> Patch 2 fixes a long standing issue that vma can be left unmerged even if
> we can for either uffd register or unregister.
>
> Many thanks to Lorenzo on either noticing this issue from the assert
> movement patch, looking at this problem, and also provided a reproducer on
> the unmerged vma issue [1].
>
> [1] https://gist.github.com/lorenzo-stoakes/a11a10f5f479e7a977fc456331266e0e
>
> This patch (of 2):
>
> It seems vma merging with uffd paths is broken with either
> register/unregister, where right now we can feed wrong parameters to
> vma_merge() and it's found by recent patch which moved asserts upwards in
> vma_merge() by Lorenzo Stoakes:
>
> https://lore.kernel.org/all/ZFunF7DmMdK05MoF@FVFF77S0Q05N.cambridge.arm.com/
>
> It's possible that "start" is contained within vma but not clamped to its
> start.  We need to convert this into either "cannot merge" case or "can
> merge" case 4 which permits subdivision of prev by assigning vma to prev.
> As we loop, each subsequent VMA will be clamped to the start.
>
> This patch will eliminate the report and make sure vma_merge() calls will
> become legal again.
>
> One thing to mention is that the "Fixes: 29417d292bd0" below is there only
> to help explain where the warning can start to trigger, the real commit to
> fix should be 69dbe6daf104.  Commit 29417d292bd0 helps us to identify the
> issue, but unfortunately we may want to keep it in Fixes too just to ease
> kernel backporters for easier tracking.
>
> Link: https://lkml.kernel.org/r/20230517190916.3429499-1-peterx@redhat.com
> Link: https://lkml.kernel.org/r/20230517190916.3429499-2-peterx@redhat.com
> Fixes: 69dbe6daf104 ("userfaultfd: use maple tree iterator to iterate VMAs")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> Closes: https://lore.kernel.org/all/ZFunF7DmMdK05MoF@FVFF77S0Q05N.cambridge.arm.com/
> Cc: Lorenzo Stoakes <lstoakes@gmail.com>
> Cc: Mike Rapoport (IBM) <rppt@kernel.org>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Jakub Acs <acsjakub@amazon.com>
> [acsjakub: contextual change - keep call to mas_next()]
> Cc: linux-mm@kvack.org
>
> ---
> This backport fixes a security issue - dangling pointer to a VMA in maple
> tree. Omitting details in this message to be brief, but happy to provide
> if requested.
>
> Since the envelope mentions series fixes 2 separate issues I hope the patch
> is acceptable on its own?
>
>
>  fs/userfaultfd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 82101a2cf933..fcf96f52b2e9 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1426,6 +1426,9 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
>  	if (prev != vma)
>  		mas_next(&mas, ULONG_MAX);
>
> +	if (vma->vm_start < start)
> +		prev = vma;
> +
>  	ret = 0;
>  	do {
>  		cond_resched();
> @@ -1603,6 +1606,9 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
>  	if (prev != vma)
>  		mas_next(&mas, ULONG_MAX);
>
> +	if (vma->vm_start < start)
> +		prev = vma;
> +
>  	ret = 0;
>  	do {
>  		cond_resched();
> --
> 2.47.1
>
>
>
> Amazon Web Services Development Center Germany GmbH
> Tamara-Danz-Str. 13
> 10243 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
> Sitz: Berlin
> Ust-ID: DE 365 538 597
>