* [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma
@ 2025-06-04 12:38 Jakub Acs
2025-06-05 11:38 ` Pratyush Yadav
2025-06-16 14:36 ` Lorenzo Stoakes
0 siblings, 2 replies; 3+ messages in thread
From: Jakub Acs @ 2025-06-04 12:38 UTC (permalink / raw)
To: stable
Cc: acsjakub, Peter Xu, Mark Rutland, Lorenzo Stoakes,
Liam R. Howlett, Mike Rapoport (IBM), Andrew Morton, Jakub Acs,
linux-mm
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma
2025-06-04 12:38 [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma Jakub Acs
@ 2025-06-05 11:38 ` Pratyush Yadav
2025-06-16 14:36 ` Lorenzo Stoakes
1 sibling, 0 replies; 3+ messages in thread
From: Pratyush Yadav @ 2025-06-05 11:38 UTC (permalink / raw)
To: Jakub Acs
Cc: stable, Peter Xu, Mark Rutland, Lorenzo Stoakes, Liam R. Howlett,
Mike Rapoport (IBM), Andrew Morton, Jakub Acs, linux-mm
Hi Jakub,
> Subject: [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma
Nit: upstream commit has "mm/uffd...". The 'M' should be lowercase.
Maybe your email client mangled the subject? Some of my tooling scans
for commit subjects to track stable patches. So it would be nice to not
change the subject without need.
On Wed, Jun 04 2025, Jakub Acs wrote:
> commit 270aa010620697fb27b8f892cc4e194bc2b7d134 upstream.
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma
2025-06-04 12:38 [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma Jakub Acs
2025-06-05 11:38 ` Pratyush Yadav
@ 2025-06-16 14:36 ` Lorenzo Stoakes
1 sibling, 0 replies; 3+ messages in thread
From: Lorenzo Stoakes @ 2025-06-16 14:36 UTC (permalink / raw)
To: Jakub Acs
Cc: stable, Peter Xu, Mark Rutland, Liam R. Howlett,
Mike Rapoport (IBM), Andrew Morton, Jakub Acs, linux-mm
-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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-16 14:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-04 12:38 [PATCH 6.1] Mm/uffd: fix vma operation where start addr cuts part of vma Jakub Acs
2025-06-05 11:38 ` Pratyush Yadav
2025-06-16 14:36 ` Lorenzo Stoakes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).