| 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 |
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;
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
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
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
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
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,
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
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 --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;
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(-)