[PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Liam R. Howlett]

commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.

This was inadvertently fixed during the removal of __vma_adjust().

When __vma_adjust() is adjusting next with a negative value (pushing
vma->vm_end lower), there would be two writes to the maple tree.  The
first write is unnecessary and uses all allocated nodes in the maple
state.  The second write is necessary but will need to allocate nodes
since the first write has used the allocated nodes.  This may be a
problem as it may not be safe to allocate at this time, such as a low
memory situation.  Fix the issue by avoiding the first write and only
write the adjusted "next" VMA.

Reported-by: John Hsu <John.Hsu@mediatek.com>
Link: https://lore.kernel.org/lkml/9cb8c599b1d7f9c1c300d1a334d5eb70ec4d7357.camel@mediatek.com/
Cc: stable@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 mm/mmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index b8af52db3bbe..bb2e0ff0ef61 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -767,7 +767,8 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	}
 	if (end != vma->vm_end) {
 		if (vma->vm_end > end) {
-			if (!insert || (insert->vm_start != end)) {
+			if ((vma->vm_end + adjust_next != end) &&
+			    (!insert || (insert->vm_start != end))) {
 				vma_mas_szero(&mas, end, vma->vm_end);
 				mas_reset(&mas);
 				VM_WARN_ON(insert &&
-- 
2.39.2

Re: [PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Greg KH]

On Thu, Jul 06, 2023 at 02:51:35PM -0400, Liam R. Howlett wrote:
> commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.
> 
> This was inadvertently fixed during the removal of __vma_adjust().
> 
> When __vma_adjust() is adjusting next with a negative value (pushing
> vma->vm_end lower), there would be two writes to the maple tree.  The
> first write is unnecessary and uses all allocated nodes in the maple
> state.  The second write is necessary but will need to allocate nodes
> since the first write has used the allocated nodes.  This may be a
> problem as it may not be safe to allocate at this time, such as a low
> memory situation.  Fix the issue by avoiding the first write and only
> write the adjusted "next" VMA.

Are you sure this is the same git id?  The one you reference above is
_VERY_ different from your 2 line change below.

And the changelog text is not the same.

confused,

greg k-h

Re: [PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Liam R. Howlett]

* Greg KH <gregkh@linuxfoundation.org> [230707 11:55]:
> On Thu, Jul 06, 2023 at 02:51:35PM -0400, Liam R. Howlett wrote:
> > commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.
> > 
> > This was inadvertently fixed during the removal of __vma_adjust().
> > 
> > When __vma_adjust() is adjusting next with a negative value (pushing
> > vma->vm_end lower), there would be two writes to the maple tree.  The
> > first write is unnecessary and uses all allocated nodes in the maple
> > state.  The second write is necessary but will need to allocate nodes
> > since the first write has used the allocated nodes.  This may be a
> > problem as it may not be safe to allocate at this time, such as a low
> > memory situation.  Fix the issue by avoiding the first write and only
> > write the adjusted "next" VMA.
> 
> Are you sure this is the same git id?  The one you reference above is
> _VERY_ different from your 2 line change below.
> 
> And the changelog text is not the same.

Yes, but I am not sure I've indicated what happened correctly.

The bug exists in the older __vma_adjust() function, but I removed
__vma_adjust() and inadvertently fixed the bug.  So the bug doesn't
exist upstream *because* of that commit:

0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")

My comment after the commit id indicates what happened, but the
documentation wasn't clear to me on how to specify what happened.

Does this answer your question?

Thanks,
Liam

Re: [PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Liam R. Howlett]

* Liam R. Howlett <Liam.Howlett@Oracle.com> [230707 12:45]:
> * Greg KH <gregkh@linuxfoundation.org> [230707 11:55]:
> > On Thu, Jul 06, 2023 at 02:51:35PM -0400, Liam R. Howlett wrote:
> > > commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.
> > > 
> > > This was inadvertently fixed during the removal of __vma_adjust().
> > > 
> > > When __vma_adjust() is adjusting next with a negative value (pushing
> > > vma->vm_end lower), there would be two writes to the maple tree.  The
> > > first write is unnecessary and uses all allocated nodes in the maple
> > > state.  The second write is necessary but will need to allocate nodes
> > > since the first write has used the allocated nodes.  This may be a
> > > problem as it may not be safe to allocate at this time, such as a low
> > > memory situation.  Fix the issue by avoiding the first write and only
> > > write the adjusted "next" VMA.
> > 
> > Are you sure this is the same git id?  The one you reference above is
> > _VERY_ different from your 2 line change below.
> > 
> > And the changelog text is not the same.
> 
> Yes, but I am not sure I've indicated what happened correctly.
> 
> The bug exists in the older __vma_adjust() function, but I removed
> __vma_adjust() and inadvertently fixed the bug.  So the bug doesn't
> exist upstream *because* of that commit:
> 
> 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
> 
> My comment after the commit id indicates what happened, but the
> documentation wasn't clear to me on how to specify what happened.
> 
> Does this answer your question?

Friendly ping on this one?

Thanks,
Liam

Re: [PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Vlastimil Babka]

On 7/12/23 02:54, Liam R. Howlett wrote:
> * Liam R. Howlett <Liam.Howlett@Oracle.com> [230707 12:45]:
>> * Greg KH <gregkh@linuxfoundation.org> [230707 11:55]:
>> > On Thu, Jul 06, 2023 at 02:51:35PM -0400, Liam R. Howlett wrote:
>> > > commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.
>> > > 
>> > > This was inadvertently fixed during the removal of __vma_adjust().
>> > > 
>> > > When __vma_adjust() is adjusting next with a negative value (pushing
>> > > vma->vm_end lower), there would be two writes to the maple tree.  The
>> > > first write is unnecessary and uses all allocated nodes in the maple
>> > > state.  The second write is necessary but will need to allocate nodes
>> > > since the first write has used the allocated nodes.  This may be a
>> > > problem as it may not be safe to allocate at this time, such as a low
>> > > memory situation.  Fix the issue by avoiding the first write and only
>> > > write the adjusted "next" VMA.
>> > 
>> > Are you sure this is the same git id?  The one you reference above is
>> > _VERY_ different from your 2 line change below.
>> > 
>> > And the changelog text is not the same.
>> 
>> Yes, but I am not sure I've indicated what happened correctly.

"commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream." is indeed not
the best indication. For stable it would mean you're backporting said
commit, which is not the case.

>> The bug exists in the older __vma_adjust() function, but I removed
>> __vma_adjust() and inadvertently fixed the bug.  So the bug doesn't
>> exist upstream *because* of that commit:
>> 
>> 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
>> 
>> My comment after the commit id indicates what happened, but the
>> documentation wasn't clear to me on how to specify what happened.

I think it's because the process discourages stable-specific fixes. However
this is the case where such approach is much simpler than  backporting
several series with non-trivial vma_merge() cleanups and subsequent
follow-up fixes...

So I agree with the exceptional stable-specific fix. Can you pinpoint a
Fixes: tag? Some of the commits introducing the maple tree?

Vlastimil

>> Does this answer your question?
> 
> Friendly ping on this one?
> 
> Thanks,
> Liam
> 

Re: [PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Liam R. Howlett]

* Vlastimil Babka <vbabka@suse.cz> [230714 10:44]:
> On 7/12/23 02:54, Liam R. Howlett wrote:
> > * Liam R. Howlett <Liam.Howlett@Oracle.com> [230707 12:45]:
> >> * Greg KH <gregkh@linuxfoundation.org> [230707 11:55]:
> >> > On Thu, Jul 06, 2023 at 02:51:35PM -0400, Liam R. Howlett wrote:
> >> > > commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.
> >> > > 
> >> > > This was inadvertently fixed during the removal of __vma_adjust().
> >> > > 
> >> > > When __vma_adjust() is adjusting next with a negative value (pushing
> >> > > vma->vm_end lower), there would be two writes to the maple tree.  The
> >> > > first write is unnecessary and uses all allocated nodes in the maple
> >> > > state.  The second write is necessary but will need to allocate nodes
> >> > > since the first write has used the allocated nodes.  This may be a
> >> > > problem as it may not be safe to allocate at this time, such as a low
> >> > > memory situation.  Fix the issue by avoiding the first write and only
> >> > > write the adjusted "next" VMA.
> >> > 
> >> > Are you sure this is the same git id?  The one you reference above is
> >> > _VERY_ different from your 2 line change below.
> >> > 
> >> > And the changelog text is not the same.
> >> 
> >> Yes, but I am not sure I've indicated what happened correctly.
> 
> "commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream." is indeed not
> the best indication. For stable it would mean you're backporting said
> commit, which is not the case.

Okay, thanks.  I am certainly not doing that.

> 
> >> The bug exists in the older __vma_adjust() function, but I removed
> >> __vma_adjust() and inadvertently fixed the bug.  So the bug doesn't
> >> exist upstream *because* of that commit:
> >> 
> >> 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
> >> 
> >> My comment after the commit id indicates what happened, but the
> >> documentation wasn't clear to me on how to specify what happened.
> 
> I think it's because the process discourages stable-specific fixes. However
> this is the case where such approach is much simpler than  backporting
> several series with non-trivial vma_merge() cleanups and subsequent
> follow-up fixes...

Thanks, and I agree.  I want to avoid that at all costs.  The chances of
introducing subtle bugs would be unacceptable for stable.

> 
> So I agree with the exceptional stable-specific fix. Can you pinpoint a
> Fixes: tag? Some of the commits introducing the maple tree?

Fixes: 3b0e81a1cdc9 ("mmap: change zeroing of maple tree in __vma_adjust()")

Thanks for the response & explanation,
Liam

Patch "mm/mmap: Fix extra maple tree write" has been added to the 6.1-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    mm/mmap: Fix extra maple tree write

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mm-mmap-fix-extra-maple-tree-write.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


From Liam.Howlett@oracle.com  Sun Jul 16 17:02:51 2023
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
Date: Thu,  6 Jul 2023 14:51:35 -0400
Subject: mm/mmap: Fix extra maple tree write
To: linux-kernel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>, "Liam R. Howlett" <Liam.Howlett@oracle.com>, John Hsu <John.Hsu@mediatek.com>, stable@vger.kernel.org, linux-mm@kvack.org
Message-ID: <20230706185135.2235532-1-Liam.Howlett@oracle.com>

From: "Liam R. Howlett" <Liam.Howlett@oracle.com>

based on commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.

This was inadvertently fixed during the removal of __vma_adjust().

When __vma_adjust() is adjusting next with a negative value (pushing
vma->vm_end lower), there would be two writes to the maple tree.  The
first write is unnecessary and uses all allocated nodes in the maple
state.  The second write is necessary but will need to allocate nodes
since the first write has used the allocated nodes.  This may be a
problem as it may not be safe to allocate at this time, such as a low
memory situation.  Fix the issue by avoiding the first write and only
write the adjusted "next" VMA.

Reported-by: John Hsu <John.Hsu@mediatek.com>
Link: https://lore.kernel.org/lkml/9cb8c599b1d7f9c1c300d1a334d5eb70ec4d7357.camel@mediatek.com/
Cc: stable@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/mmap.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -767,7 +767,8 @@ int __vma_adjust(struct vm_area_struct *
 	}
 	if (end != vma->vm_end) {
 		if (vma->vm_end > end) {
-			if (!insert || (insert->vm_start != end)) {
+			if ((vma->vm_end + adjust_next != end) &&
+			    (!insert || (insert->vm_start != end))) {
 				vma_mas_szero(&mas, end, vma->vm_end);
 				mas_reset(&mas);
 				VM_WARN_ON(insert &&


Patches currently in stable-queue which might be from Liam.Howlett@oracle.com are

queue-6.1/mm-mmap-fix-extra-maple-tree-write.patch
queue-6.1/mm-mmap-fix-vm_locked-check-in-do_vmi_align_munmap.patch

Re: [PATCH stable v6.1] mm/mmap: Fix extra maple tree write

[Greg KH]

On Tue, Jul 11, 2023 at 08:54:42PM -0400, Liam R. Howlett wrote:
> * Liam R. Howlett <Liam.Howlett@Oracle.com> [230707 12:45]:
> > * Greg KH <gregkh@linuxfoundation.org> [230707 11:55]:
> > > On Thu, Jul 06, 2023 at 02:51:35PM -0400, Liam R. Howlett wrote:
> > > > commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.
> > > > 
> > > > This was inadvertently fixed during the removal of __vma_adjust().
> > > > 
> > > > When __vma_adjust() is adjusting next with a negative value (pushing
> > > > vma->vm_end lower), there would be two writes to the maple tree.  The
> > > > first write is unnecessary and uses all allocated nodes in the maple
> > > > state.  The second write is necessary but will need to allocate nodes
> > > > since the first write has used the allocated nodes.  This may be a
> > > > problem as it may not be safe to allocate at this time, such as a low
> > > > memory situation.  Fix the issue by avoiding the first write and only
> > > > write the adjusted "next" VMA.
> > > 
> > > Are you sure this is the same git id?  The one you reference above is
> > > _VERY_ different from your 2 line change below.
> > > 
> > > And the changelog text is not the same.
> > 
> > Yes, but I am not sure I've indicated what happened correctly.
> > 
> > The bug exists in the older __vma_adjust() function, but I removed
> > __vma_adjust() and inadvertently fixed the bug.  So the bug doesn't
> > exist upstream *because* of that commit:
> > 
> > 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
> > 
> > My comment after the commit id indicates what happened, but the
> > documentation wasn't clear to me on how to specify what happened.
> > 
> > Does this answer your question?
> 
> Friendly ping on this one?

Now queued up, thanks.

greg k-h

Patch "mm/mmap: Fix extra maple tree write" has been added to the 5.4-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    mm/mmap: Fix extra maple tree write

to the 5.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mm-mmap-fix-extra-maple-tree-write.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


From Liam.Howlett@oracle.com  Sun Jul 16 17:02:51 2023
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
Date: Thu,  6 Jul 2023 14:51:35 -0400
Subject: mm/mmap: Fix extra maple tree write
To: linux-kernel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>, "Liam R. Howlett" <Liam.Howlett@oracle.com>, John Hsu <John.Hsu@mediatek.com>, stable@vger.kernel.org, linux-mm@kvack.org
Message-ID: <20230706185135.2235532-1-Liam.Howlett@oracle.com>

From: "Liam R. Howlett" <Liam.Howlett@oracle.com>

based on commit 0503ea8f5ba73eb3ab13a81c1eefbaf51405385a upstream.

This was inadvertently fixed during the removal of __vma_adjust().

When __vma_adjust() is adjusting next with a negative value (pushing
vma->vm_end lower), there would be two writes to the maple tree.  The
first write is unnecessary and uses all allocated nodes in the maple
state.  The second write is necessary but will need to allocate nodes
since the first write has used the allocated nodes.  This may be a
problem as it may not be safe to allocate at this time, such as a low
memory situation.  Fix the issue by avoiding the first write and only
write the adjusted "next" VMA.

Reported-by: John Hsu <John.Hsu@mediatek.com>
Link: https://lore.kernel.org/lkml/9cb8c599b1d7f9c1c300d1a334d5eb70ec4d7357.camel@mediatek.com/
Cc: stable@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/mmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index b8af52db3bbe..bb2e0ff0ef61 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -767,7 +767,8 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	}
 	if (end != vma->vm_end) {
 		if (vma->vm_end > end) {
-			if (!insert || (insert->vm_start != end)) {
+			if ((vma->vm_end + adjust_next != end) &&
+			    (!insert || (insert->vm_start != end))) {
 				vma_mas_szero(&mas, end, vma->vm_end);
 				mas_reset(&mas);
 				VM_WARN_ON(insert &&
-- 
2.39.2



Patches currently in stable-queue which might be from Liam.Howlett@oracle.com are

queue-5.4/mm-mmap-fix-extra-maple-tree-write.patch