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 <[email protected]>
Reported-and-tested-by: Fabian Vogt <[email protected]>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
Signed-off-by: Vlastimil Babka <[email protected]>
Cc: <[email protected]>
---
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(-)
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;
--
2.40.0
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 <[email protected]>
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 <[email protected]>
Reported-and-tested-by: Fabian Vogt <[email protected]>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
Signed-off-by: Vlastimil Babka <[email protected]>
Cc: <[email protected]>
---
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;
--
2.40.0
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 <[email protected]>
> Reported-and-tested-by: Fabian Vogt <[email protected]>
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
> Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
> Signed-off-by: Vlastimil Babka <[email protected]>
> Cc: <[email protected]>
> ---
> 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
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 <[email protected]>
>> Reported-and-tested-by: Fabian Vogt <[email protected]>
>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
>> Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
>> Signed-off-by: Vlastimil Babka <[email protected]>
>> Cc: <[email protected]>
>> ---
>> 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
On Thu, Apr 27, 2023 at 7:39 AM Vlastimil Babka <[email protected]> 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
Hi Vlastimil,
On Thu, Apr 27, 2023 at 8:12 AM Linus Torvalds
<[email protected]> 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
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 <[email protected]>
>> Reported-and-tested-by: Fabian Vogt <[email protected]>
>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1210903
>> Fixes: 0503ea8f5ba7 ("mm/mmap: remove __vma_adjust()")
>> Signed-off-by: Vlastimil Babka <[email protected]>
>> Cc: <[email protected]>
>> ---
>> 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 <[email protected]>
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,
--
js
suse labs
On 4/28/23 04:53, Linus Torvalds wrote:
> Hi Vlastimil,
Hi Linus,
> On Thu, Apr 27, 2023 at 8:12 AM Linus Torvalds
> <[email protected]> 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
On Thu, Apr 27, 2023 at 08:12:40AM -0700, Linus Torvalds wrote:
> On Thu, Apr 27, 2023 at 7:39 AM Vlastimil Babka <[email protected]> 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