diff mbox series

[for,v6.3,regression] mm/mremap: fix vm_pgoff in vma_merge() case 3

Message ID 20230427140959.27655-1-vbabka@suse.cz (mailing list archive)
State New
Headers show
Series [for,v6.3,regression] mm/mremap: fix vm_pgoff in vma_merge() case 3 | expand

Commit Message

Vlastimil Babka April 27, 2023, 2:09 p.m. UTC
After upgrading build guests to v6.3, rpm started segfaulting for
specific packages, which was bisected to commit 0503ea8f5ba7 ("mm/mmap:
remove __vma_adjust()"). rpm is doing many mremap() operations with file
mappings of its db. The problem is that in vma_merge() case 3 (we merge
with the next vma, expanding it downwards) vm_pgoff is not adjusted as
it should when vm_start changes. As a result the rpm process most likely
sees data from the wrong offset of the file. Fix the vm_pgoff
calculation.

For case 8 this is a non-functional change as the resulting vm_pgoff is
the same.

Reported-and-bisected-by: Jiri Slaby <jirislaby@kernel.org>
Reported-and-tested-by: Fabian Vogt <fvogt@suse.com>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>
---
Hi, I'm sending this patch on top of v6.3 as I think it should be
applied and backported to 6.3-stable rather sooner than later.
This means there would be a small conflict when merging mm/mm-stable
later. Alternatively it could be added to mm/mm-stable and upcoming 6.4
pull request, but then the stable backport would need adjustment.
It's up to Linus and Andrew.

#regzbot introduced: 0503ea8f5ba7 https://bugzilla.suse.com/show_bug.cgi?id=1210903

 mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vlastimil Babka April 27, 2023, 2:15 p.m. UTC | #1
On 4/27/23 16:09, Vlastimil Babka wrote:
> |later. Alternatively it could be added to mm/mm-stable and upcoming 6.4
> pull request, but then the stable backport would need adjustment. It's up to
> Linus and Andrew. |

This version applies on mm/mm-stable. Paragraph about case 8 is removed
as that case sets vma_pgoff explicitly itself.

----8<----
From dea6d87bdad1fbb21f987dba96c55195fb88e7b4 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Thu, 27 Apr 2023 15:28:41 +0200
Subject: [PATCH] mm/mremap: fix vm_pgoff in vma_merge() case 3

After upgrading build guests to v6.3, rpm started segfaulting for
specific packages, which was bisected to commit 0503ea8f5ba7 ("mm/mmap:
remove __vma_adjust()"). rpm is doing many mremap() operations with file
mappings of its db. The problem is that in vma_merge() case 3 (we merge
with the next vma, expanding it downwards) vm_pgoff is not adjusted as
it should when vm_start changes. As a result the rpm process most likely
sees data from the wrong offset of the file. Fix the vm_pgoff
calculation.

Reported-and-bisected-by: Jiri Slaby <jirislaby@kernel.org>
Reported-and-tested-by: Fabian Vogt <fvogt@suse.com>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>
---
 mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 536bbb8fa0ae..5522130ae606 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1008,7 +1008,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
 			vma = next;			/* case 3 */
 			vma_start = addr;
 			vma_end = next->vm_end;
-			vma_pgoff = next->vm_pgoff;
+			vma_pgoff = next->vm_pgoff - pglen;
 			if (curr) {			/* case 8 */
 				vma_pgoff = curr->vm_pgoff;
 				remove = curr;
Greg KH April 27, 2023, 2:27 p.m. UTC | #2
On Thu, Apr 27, 2023 at 04:09:59PM +0200, Vlastimil Babka wrote:
> After upgrading build guests to v6.3, rpm started segfaulting for
> specific packages, which was bisected to commit 0503ea8f5ba7 ("mm/mmap:
> remove __vma_adjust()"). rpm is doing many mremap() operations with file
> mappings of its db. The problem is that in vma_merge() case 3 (we merge
> with the next vma, expanding it downwards) vm_pgoff is not adjusted as
> it should when vm_start changes. As a result the rpm process most likely
> sees data from the wrong offset of the file. Fix the vm_pgoff
> calculation.
> 
> For case 8 this is a non-functional change as the resulting vm_pgoff is
> the same.
> 
> Reported-and-bisected-by: Jiri Slaby <jirislaby@kernel.org>
> Reported-and-tested-by: Fabian Vogt <fvogt@suse.com>
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
> Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Cc: <stable@vger.kernel.org>
> ---
> Hi, I'm sending this patch on top of v6.3 as I think it should be
> applied and backported to 6.3-stable rather sooner than later.
> This means there would be a small conflict when merging mm/mm-stable
> later. Alternatively it could be added to mm/mm-stable and upcoming 6.4
> pull request, but then the stable backport would need adjustment.
> It's up to Linus and Andrew.

That's not how the stable tree works, sorry, it needs to be in Linus's
tree _first_.

thanks,

greg k-h
Vlastimil Babka April 27, 2023, 2:39 p.m. UTC | #3
On 4/27/23 16:27, Greg KH wrote:
> On Thu, Apr 27, 2023 at 04:09:59PM +0200, Vlastimil Babka wrote:
>> After upgrading build guests to v6.3, rpm started segfaulting for
>> specific packages, which was bisected to commit 0503ea8f5ba7 ("mm/mmap:
>> remove __vma_adjust()"). rpm is doing many mremap() operations with file
>> mappings of its db. The problem is that in vma_merge() case 3 (we merge
>> with the next vma, expanding it downwards) vm_pgoff is not adjusted as
>> it should when vm_start changes. As a result the rpm process most likely
>> sees data from the wrong offset of the file. Fix the vm_pgoff
>> calculation.
>> 
>> For case 8 this is a non-functional change as the resulting vm_pgoff is
>> the same.
>> 
>> Reported-and-bisected-by: Jiri Slaby <jirislaby@kernel.org>
>> Reported-and-tested-by: Fabian Vogt <fvogt@suse.com>
>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
>> Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>> Cc: <stable@vger.kernel.org>
>> ---
>> Hi, I'm sending this patch on top of v6.3 as I think it should be
>> applied and backported to 6.3-stable rather sooner than later.
>> This means there would be a small conflict when merging mm/mm-stable
>> later. Alternatively it could be added to mm/mm-stable and upcoming 6.4
>> pull request, but then the stable backport would need adjustment.
>> It's up to Linus and Andrew.
> 
> That's not how the stable tree works, sorry, it needs to be in Linus's
> tree _first_.

Sorry, I wasn't clear what I meant here. I didn't intend to bypass that
stable rule that I'm aware of, just that it might be desirable to get this
fix to Linus's tree faster so that stable tree can also take it soon.

> thanks,
> 
> greg k-h
Linus Torvalds April 27, 2023, 3:12 p.m. UTC | #4
On Thu, Apr 27, 2023 at 7:39 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> Sorry, I wasn't clear what I meant here. I didn't intend to bypass that
> stable rule that I'm aware of, just that it might be desirable to get this
> fix to Linus's tree faster so that stable tree can also take it soon.

Ack. It's in my tree as commit 7e7757876f25 right now (not pushed out
yet, will do the usual build tests and look around for other things
pending).

                 Linus
Linus Torvalds April 28, 2023, 2:53 a.m. UTC | #5
Hi Vlastimil,

On Thu, Apr 27, 2023 at 8:12 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Ack. It's in my tree as commit 7e7757876f25 right now (not pushed out
> yet, will do the usual build tests and look around for other things
> pending).

Gaah. I just merged Andrew's MM tree, and while it had a lot of small
conflicts (and the ext4 ones were annoying semantic ones), the only
one that was in *confusing* code was the one introduced by this
one-liner fix.

I'm pretty sure I did the right thing, particularly given your other
patch for the mm tree, but please humor me and take a look at it?

That 'vma_merge()' function is the function from hell.

I haven't pushed out yet because it's still going through my build
tests, but it should be out soon.

                       Linus
Jiri Slaby April 28, 2023, 6:15 a.m. UTC | #6
On 27. 04. 23, 16:27, Greg KH wrote:
> On Thu, Apr 27, 2023 at 04:09:59PM +0200, Vlastimil Babka wrote:
>> After upgrading build guests to v6.3, rpm started segfaulting for
>> specific packages, which was bisected to commit 0503ea8f5ba7 ("mm/mmap:
>> remove __vma_adjust()"). rpm is doing many mremap() operations with file
>> mappings of its db. The problem is that in vma_merge() case 3 (we merge
>> with the next vma, expanding it downwards) vm_pgoff is not adjusted as
>> it should when vm_start changes. As a result the rpm process most likely
>> sees data from the wrong offset of the file. Fix the vm_pgoff
>> calculation.
>>
>> For case 8 this is a non-functional change as the resulting vm_pgoff is
>> the same.
>>
>> Reported-and-bisected-by: Jiri Slaby <jirislaby@kernel.org>
>> Reported-and-tested-by: Fabian Vogt <fvogt@suse.com>
>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
>> Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>> Cc: <stable@vger.kernel.org>
>> ---
>> Hi, I'm sending this patch on top of v6.3 as I think it should be
>> applied and backported to 6.3-stable rather sooner than later.
>> This means there would be a small conflict when merging mm/mm-stable
>> later. Alternatively it could be added to mm/mm-stable and upcoming 6.4
>> pull request, but then the stable backport would need adjustment.
>> It's up to Linus and Andrew.
> 
> That's not how the stable tree works, sorry, it needs to be in Linus's
> tree _first_.

In upstream as:
commit 7e7757876f258d99266e7b3c559639289a2a45fe
Author: Vlastimil Babka <vbabka@suse.cz>
Date:   Thu Apr 27 16:09:59 2023 +0200

     mm/mremap: fix vm_pgoff in vma_merge() case 3

Please queue for 6.3.1.

thanks,
Vlastimil Babka April 28, 2023, 7:13 a.m. UTC | #7
On 4/28/23 04:53, Linus Torvalds wrote:
> Hi Vlastimil,

Hi Linus,

> On Thu, Apr 27, 2023 at 8:12 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Ack. It's in my tree as commit 7e7757876f25 right now (not pushed out
>> yet, will do the usual build tests and look around for other things
>> pending).
> 
> Gaah. I just merged Andrew's MM tree, and while it had a lot of small
> conflicts (and the ext4 ones were annoying semantic ones), the only
> one that was in *confusing* code was the one introduced by this
> one-liner fix.
> 
> I'm pretty sure I did the right thing, particularly given your other
> patch for the mm tree, but please humor me and take a look at it?

Sure, took a look and looks correct to me, thanks!

> That 'vma_merge()' function is the function from hell.

Yeah, unfortunately. But despite the bugs, I believe Liam's changes in 6.3
improved it a lot, as with __vma_adjust() it was much worse (it did e.g.
things like "swap(vma, expand)" in some cases). And hopefully the cleanups
from me and Lorenzo in the mm 6.4 pull request improved readability too,
even though it made the merge tricky.

> I haven't pushed out yet because it's still going through my build
> tests, but it should be out soon.
> 
>                        Linus
Greg Kroah-Hartman April 28, 2023, 7:36 a.m. UTC | #8
On Thu, Apr 27, 2023 at 08:12:40AM -0700, Linus Torvalds wrote:
> On Thu, Apr 27, 2023 at 7:39 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > Sorry, I wasn't clear what I meant here. I didn't intend to bypass that
> > stable rule that I'm aware of, just that it might be desirable to get this
> > fix to Linus's tree faster so that stable tree can also take it soon.
> 
> Ack. It's in my tree as commit 7e7757876f25 right now (not pushed out
> yet, will do the usual build tests and look around for other things
> pending).

Thanks, I've grabbed it now for the 6.3.y tree.

greg k-h
diff mbox series

Patch

diff --git a/mm/mmap.c b/mm/mmap.c
index d5475fbf5729..eefa6f0cda28 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -978,7 +978,7 @@  struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
 			vma = next;			/* case 3 */
 			vma_start = addr;
 			vma_end = next->vm_end;
-			vma_pgoff = mid->vm_pgoff;
+			vma_pgoff = next->vm_pgoff - pglen;
 			err = 0;
 			if (mid != next) {		/* case 8 */
 				remove = mid;