linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
@ 2024-06-13 23:40 Bert Karwatzki
  2024-06-14  0:03 ` Andrew Morton
  2024-06-14  7:54 ` Jiri Olsa
  0 siblings, 2 replies; 13+ messages in thread
From: Bert Karwatzki @ 2024-06-13 23:40 UTC (permalink / raw)
  To: Liam R. Howlett, linux-mm, linux-kernel, Andrew Morton,
	linux-next

Since linux-next-20240613 firefox-esr crashes after several minutes of browsing
giving the following error messages in dmesg:
[ T2343] BUG: Bad rss-counter state mm:00000000babe0c39 type:MM_ANONPAGES val:86
[ T4063] show_signal_msg: 16 callbacks suppressed
[ T4063] Isolated Web Co[4063]: segfault at 396d1686c000 ip 0000396d1686c000 sp
00007ffd767b30a8 error 14 likely on CPU 7 (core 3, socket 0)
[ T4063] Code: Unable to access opcode bytes at 0x396d1686bfd6.
[ T4211] BUG: Bad rss-counter state mm:00000000cd9fc541 type:MM_ANONPAGES
val:817
[ T3798] BUG: Bad rss-counter state mm:00000000432d87c2 type:MM_ANONPAGES
val:181
[ T5548] BUG: Bad rss-counter state mm:00000000034aa27a type:MM_ANONPAGES
val:242
[ T3823] BUG: Bad rss-counter state mm:0000000099734197 type:MM_ANONPAGES
val:137
[    T1] BUG: Bad rss-counter state mm:000000005e5e2f2f type:MM_ANONPAGES val:28

(these are the error messages of several crashes and the error seems to affect
other processes, too (T1))

The crash can be provoked to appear in ~1min by opening large numbers of tabs in
firefox-esr (by holding pressing ctrl+t for some time). With this I bisected the
error to commit "1c29a32ce65f mm/mmap: use split munmap calls for MAP_FIXED" and
reverting this commit in linux-next-20240613 fixes the issue for me.

Bert Karwatzki

PS. Please CC me when answering, I'm not subscribed to the lists.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-13 23:40 Bert Karwatzki
@ 2024-06-14  0:03 ` Andrew Morton
  2024-06-14  8:26   ` Bert Karwatzki
                     ` (2 more replies)
  2024-06-14  7:54 ` Jiri Olsa
  1 sibling, 3 replies; 13+ messages in thread
From: Andrew Morton @ 2024-06-14  0:03 UTC (permalink / raw)
  To: Bert Karwatzki; +Cc: Liam R. Howlett, linux-mm, linux-kernel, linux-next

On Fri, 14 Jun 2024 01:40:54 +0200 Bert Karwatzki <spasswolf@web.de> wrote:

> Since linux-next-20240613 firefox-esr crashes after several minutes of browsing
> giving the following error messages in dmesg:
> [ T2343] BUG: Bad rss-counter state mm:00000000babe0c39 type:MM_ANONPAGES val:86
> [ T4063] show_signal_msg: 16 callbacks suppressed
> [ T4063] Isolated Web Co[4063]: segfault at 396d1686c000 ip 0000396d1686c000 sp
> 00007ffd767b30a8 error 14 likely on CPU 7 (core 3, socket 0)
> [ T4063] Code: Unable to access opcode bytes at 0x396d1686bfd6.
> [ T4211] BUG: Bad rss-counter state mm:00000000cd9fc541 type:MM_ANONPAGES
> val:817
> [ T3798] BUG: Bad rss-counter state mm:00000000432d87c2 type:MM_ANONPAGES
> val:181
> [ T5548] BUG: Bad rss-counter state mm:00000000034aa27a type:MM_ANONPAGES
> val:242
> [ T3823] BUG: Bad rss-counter state mm:0000000099734197 type:MM_ANONPAGES
> val:137
> [    T1] BUG: Bad rss-counter state mm:000000005e5e2f2f type:MM_ANONPAGES val:28

Let's hope Linus doesn't read this.  Why are we nuking the entire
planet just because some counter went wonky?

> (these are the error messages of several crashes and the error seems to affect
> other processes, too (T1))
> 
> The crash can be provoked to appear in ~1min by opening large numbers of tabs in
> firefox-esr (by holding pressing ctrl+t for some time). With this I bisected the
> error to commit "1c29a32ce65f mm/mmap: use split munmap calls for MAP_FIXED" and
> reverting this commit in linux-next-20240613 fixes the issue for me.

Thanks, that must have taken a lot of time.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-13 23:40 Bert Karwatzki
  2024-06-14  0:03 ` Andrew Morton
@ 2024-06-14  7:54 ` Jiri Olsa
  2024-06-14 12:31   ` Liam R. Howlett
  1 sibling, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2024-06-14  7:54 UTC (permalink / raw)
  To: Bert Karwatzki
  Cc: Liam R. Howlett, linux-mm, linux-kernel, Andrew Morton,
	linux-next

On Fri, Jun 14, 2024 at 01:40:54AM +0200, Bert Karwatzki wrote:
> Since linux-next-20240613 firefox-esr crashes after several minutes of browsing
> giving the following error messages in dmesg:
> [ T2343] BUG: Bad rss-counter state mm:00000000babe0c39 type:MM_ANONPAGES val:86
> [ T4063] show_signal_msg: 16 callbacks suppressed
> [ T4063] Isolated Web Co[4063]: segfault at 396d1686c000 ip 0000396d1686c000 sp
> 00007ffd767b30a8 error 14 likely on CPU 7 (core 3, socket 0)
> [ T4063] Code: Unable to access opcode bytes at 0x396d1686bfd6.
> [ T4211] BUG: Bad rss-counter state mm:00000000cd9fc541 type:MM_ANONPAGES
> val:817
> [ T3798] BUG: Bad rss-counter state mm:00000000432d87c2 type:MM_ANONPAGES
> val:181
> [ T5548] BUG: Bad rss-counter state mm:00000000034aa27a type:MM_ANONPAGES
> val:242
> [ T3823] BUG: Bad rss-counter state mm:0000000099734197 type:MM_ANONPAGES
> val:137
> [    T1] BUG: Bad rss-counter state mm:000000005e5e2f2f type:MM_ANONPAGES val:28
> 
> (these are the error messages of several crashes and the error seems to affect
> other processes, too (T1))
> 
> The crash can be provoked to appear in ~1min by opening large numbers of tabs in
> firefox-esr (by holding pressing ctrl+t for some time). With this I bisected the
> error to commit "1c29a32ce65f mm/mmap: use split munmap calls for MAP_FIXED" and
> reverting this commit in linux-next-20240613 fixes the issue for me.

+1, bpf selftests are failing for me because mmap fails with:
  mmap(0x7f9361bc9000, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 4, 0) = -1 EBUSY (Device or resource busy)

did not get to the cause, but reverting the 1c29a32ce65f fixes it for me

thanks,
jirka

> 
> Bert Karwatzki
> 
> PS. Please CC me when answering, I'm not subscribed to the lists.
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-14  0:03 ` Andrew Morton
@ 2024-06-14  8:26   ` Bert Karwatzki
  2024-06-14  8:37   ` Bert Karwatzki
  2024-06-14 12:30   ` Liam R. Howlett
  2 siblings, 0 replies; 13+ messages in thread
From: Bert Karwatzki @ 2024-06-14  8:26 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Liam R. Howlett, linux-mm, linux-kernel, linux-next

Am Donnerstag, dem 13.06.2024 um 17:03 -0700 schrieb Andrew Morton:
> Let's hope Linus doesn't read this.  Why are we nuking the entire
> planet just because some counter went wonky?

It's not just the wonky counter, the error always comes with a segfault which
causes a firefox tab to crash, though I've not yet figured out how this is
related to the BUG message or commit 1c29a32ce.

[  179.393488] [   T2278] show_signal_msg: 16 callbacks suppressed
[  179.393492] [   T2278] Privileged Cont[2278]: segfault at 22cddf91d3a0 ip
000022cde1aad010 sp 00007ffc616851a8 error 7 likely on CPU 15 (core 7, socket 0)
[  179.393504] [   T2278] Code: Unable to access opcode bytes at 0x22cde1aacfe6.
[  179.498173] [   T2289] BUG: Bad rss-counter state mm:00000000d0a3f682
type:MM_ANONPAGES val:1885

Bert Karwatzki



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-14  0:03 ` Andrew Morton
  2024-06-14  8:26   ` Bert Karwatzki
@ 2024-06-14  8:37   ` Bert Karwatzki
  2024-06-14 12:30   ` Liam R. Howlett
  2 siblings, 0 replies; 13+ messages in thread
From: Bert Karwatzki @ 2024-06-14  8:37 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Liam R. Howlett, linux-mm, linux-kernel, linux-next

Am Donnerstag, dem 13.06.2024 um 17:03 -0700 schrieb Andrew Morton:
> Let's hope Linus doesn't read this.  Why are we nuking the entire
> planet just because some counter went wonky?

It's not just the wonky counter, the error always comes with a segfault which
causes a firefox tab to crash, though I've not yet figured out how this is
related to the BUG message or commit 1c29a32ce.

[  179.393488] [   T2278] show_signal_msg: 16 callbacks suppressed
[  179.393492] [   T2278] Privileged Cont[2278]: segfault at 22cddf91d3a0 ip
000022cde1aad010 sp 00007ffc616851a8 error 7 likely on CPU 15 (core 7, socket 0)
[  179.393504] [   T2278] Code: Unable to access opcode bytes at 0x22cde1aacfe6.
[  179.498173] [   T2289] BUG: Bad rss-counter state mm:00000000d0a3f682
type:MM_ANONPAGES val:1885

Bert Karwatzki


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-14  0:03 ` Andrew Morton
  2024-06-14  8:26   ` Bert Karwatzki
  2024-06-14  8:37   ` Bert Karwatzki
@ 2024-06-14 12:30   ` Liam R. Howlett
  2024-06-14 17:21     ` Andrew Morton
  2 siblings, 1 reply; 13+ messages in thread
From: Liam R. Howlett @ 2024-06-14 12:30 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Bert Karwatzki, linux-mm, linux-kernel, linux-next

* Andrew Morton <akpm@linux-foundation.org> [240613 20:03]:
> On Fri, 14 Jun 2024 01:40:54 +0200 Bert Karwatzki <spasswolf@web.de> wrote:
> 
> > Since linux-next-20240613 firefox-esr crashes after several minutes of browsing
> > giving the following error messages in dmesg:
> > [ T2343] BUG: Bad rss-counter state mm:00000000babe0c39 type:MM_ANONPAGES val:86
> > [ T4063] show_signal_msg: 16 callbacks suppressed
> > [ T4063] Isolated Web Co[4063]: segfault at 396d1686c000 ip 0000396d1686c000 sp
> > 00007ffd767b30a8 error 14 likely on CPU 7 (core 3, socket 0)
> > [ T4063] Code: Unable to access opcode bytes at 0x396d1686bfd6.
> > [ T4211] BUG: Bad rss-counter state mm:00000000cd9fc541 type:MM_ANONPAGES
> > val:817
> > [ T3798] BUG: Bad rss-counter state mm:00000000432d87c2 type:MM_ANONPAGES
> > val:181
> > [ T5548] BUG: Bad rss-counter state mm:00000000034aa27a type:MM_ANONPAGES
> > val:242
> > [ T3823] BUG: Bad rss-counter state mm:0000000099734197 type:MM_ANONPAGES
> > val:137
> > [    T1] BUG: Bad rss-counter state mm:000000005e5e2f2f type:MM_ANONPAGES val:28
> 
> Let's hope Linus doesn't read this.  Why are we nuking the entire
> planet just because some counter went wonky?

I think I know what's going on, and it's more than just the counters
being off here.  The counters are the symptom of what is happening.

> 
> > (these are the error messages of several crashes and the error seems to affect
> > other processes, too (T1))
> > 
> > The crash can be provoked to appear in ~1min by opening large numbers of tabs in
> > firefox-esr (by holding pressing ctrl+t for some time). With this I bisected the
> > error to commit "1c29a32ce65f mm/mmap: use split munmap calls for MAP_FIXED" and
> > reverting this commit in linux-next-20240613 fixes the issue for me.
> 
> Thanks, that must have taken a lot of time.

Yes, thank you for all that work and apologies in creating this
frustrating situation.

Andrew, please drop the set from your branch.  I need to write some more
tests, but I suspect I will need to do some work around the vma_merge()
function, which is never a fun endeavor.

Regards,
Liam


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-14  7:54 ` Jiri Olsa
@ 2024-06-14 12:31   ` Liam R. Howlett
  0 siblings, 0 replies; 13+ messages in thread
From: Liam R. Howlett @ 2024-06-14 12:31 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Bert Karwatzki, linux-mm, linux-kernel, Andrew Morton, linux-next

* Jiri Olsa <olsajiri@gmail.com> [240614 03:54]:
> On Fri, Jun 14, 2024 at 01:40:54AM +0200, Bert Karwatzki wrote:
> > Since linux-next-20240613 firefox-esr crashes after several minutes of browsing
> > giving the following error messages in dmesg:
> > [ T2343] BUG: Bad rss-counter state mm:00000000babe0c39 type:MM_ANONPAGES val:86
> > [ T4063] show_signal_msg: 16 callbacks suppressed
> > [ T4063] Isolated Web Co[4063]: segfault at 396d1686c000 ip 0000396d1686c000 sp
> > 00007ffd767b30a8 error 14 likely on CPU 7 (core 3, socket 0)
> > [ T4063] Code: Unable to access opcode bytes at 0x396d1686bfd6.
> > [ T4211] BUG: Bad rss-counter state mm:00000000cd9fc541 type:MM_ANONPAGES
> > val:817
> > [ T3798] BUG: Bad rss-counter state mm:00000000432d87c2 type:MM_ANONPAGES
> > val:181
> > [ T5548] BUG: Bad rss-counter state mm:00000000034aa27a type:MM_ANONPAGES
> > val:242
> > [ T3823] BUG: Bad rss-counter state mm:0000000099734197 type:MM_ANONPAGES
> > val:137
> > [    T1] BUG: Bad rss-counter state mm:000000005e5e2f2f type:MM_ANONPAGES val:28
> > 
> > (these are the error messages of several crashes and the error seems to affect
> > other processes, too (T1))
> > 
> > The crash can be provoked to appear in ~1min by opening large numbers of tabs in
> > firefox-esr (by holding pressing ctrl+t for some time). With this I bisected the
> > error to commit "1c29a32ce65f mm/mmap: use split munmap calls for MAP_FIXED" and
> > reverting this commit in linux-next-20240613 fixes the issue for me.
> 
> +1, bpf selftests are failing for me because mmap fails with:
>   mmap(0x7f9361bc9000, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 4, 0) = -1 EBUSY (Device or resource busy)
> 
> did not get to the cause, but reverting the 1c29a32ce65f fixes it for me
> 

Thanks for the information, this sounds like a much easier way to
recreate the issue.

Regards,
Liam


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-06-14 12:30   ` Liam R. Howlett
@ 2024-06-14 17:21     ` Andrew Morton
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Morton @ 2024-06-14 17:21 UTC (permalink / raw)
  To: Liam R. Howlett; +Cc: Bert Karwatzki, linux-mm, linux-kernel, linux-next

On Fri, 14 Jun 2024 08:30:20 -0400 "Liam R. Howlett" <Liam.Howlett@oracle.com> wrote:

> > 
> > > (these are the error messages of several crashes and the error seems to affect
> > > other processes, too (T1))
> > > 
> > > The crash can be provoked to appear in ~1min by opening large numbers of tabs in
> > > firefox-esr (by holding pressing ctrl+t for some time). With this I bisected the
> > > error to commit "1c29a32ce65f mm/mmap: use split munmap calls for MAP_FIXED" and
> > > reverting this commit in linux-next-20240613 fixes the issue for me.
> > 
> > Thanks, that must have taken a lot of time.
> 
> Yes, thank you for all that work and apologies in creating this
> frustrating situation.
> 
> Andrew, please drop the set from your branch.  I need to write some more
> tests, but I suspect I will need to do some work around the vma_merge()
> function, which is never a fun endeavor.

Dropped, thanks.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
@ 2024-06-21 19:32 Bert Karwatzki
  0 siblings, 0 replies; 13+ messages in thread
From: Bert Karwatzki @ 2024-06-21 19:32 UTC (permalink / raw)
  To: Liam R . Howlett
  Cc: Bert Karwatzki, Andrew Morton, Jiri Olsa, linux-mm, linux-kernel,
	linux-next

I did some experiments on the rss counter bug. The next patch is made for linux-next-20240613
with commit 1c29a32ce65f4cd0f1c0f9 reverted. Then I simply inlined the code of do_vmi_unmap()
and do_vmi_align_munmap() into mmap_region(). This version of the code works fine and does not
show the rss counter bug.

diff --git a/mm/mmap.c b/mm/mmap.c
index f95af72ddc9f..0f020c535c83 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -733,7 +733,6 @@ int vma_expand(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	vma_iter_store(vmi, vma);

 	vma_complete(&vp, vmi, vma->vm_mm);
-	validate_mm(vma->vm_mm);
 	return 0;

 nomem:
@@ -2911,6 +2910,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 	struct vm_area_struct *next, *prev, *merge;
 	pgoff_t pglen = len >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	struct vma_munmap_struct vms;
+	struct ma_state mas_detach;
 	unsigned long end = addr + len;
 	unsigned long merge_start = addr, merge_end = end;
 	bool writable_file_mapping = false;
@@ -2933,12 +2934,46 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 			return -ENOMEM;
 	}

-	/* Unmap any existing mapping in the area */
-	error = do_vmi_munmap(&vmi, mm, addr, len, uf, false);
-	if (error == -EPERM)
-		return error;
-	else if (error)
-		return -ENOMEM;
+	/* Find the first overlapping VMA */
+	vma = vma_find(&vmi, end);
+	if (vma) {
+		struct maple_tree mt_detach;
+
+		/*
+		 * Check if memory is sealed before arch_unmap.
+		 * Prevent unmapping a sealed VMA.
+		 * can_modify_mm assumes we have acquired the lock on MM.
+		 */
+		if (unlikely(!can_modify_mm(mm, addr, end))) {
+			return -EPERM;
+		}
+
+		 /* arch_unmap() might do unmaps itself.  */
+		arch_unmap(mm, addr, end);
+
+		mt_init_flags(&mt_detach, vmi.mas.tree->ma_flags & MT_FLAGS_LOCK_MASK);
+		mt_on_stack(mt_detach);
+		mas_init(&mas_detach, &mt_detach, 0);
+
+		init_vma_munmap(&vms, &vmi, vma, addr, end, uf, false);
+		error = vms_gather_munmap_vmas(&vms, &mas_detach);
+		if (error) {
+			validate_mm(mm);
+			return -ENOMEM;
+		}
+
+		vma = NULL;
+		error = vma_iter_clear_gfp(&vmi, addr, end, GFP_KERNEL);
+		if (error) {
+			abort_munmap_vmas(&mas_detach);
+			return -ENOMEM;
+		}
+
+		/* Point of no return */
+		vms_complete_munmap_vmas(&vms, &mas_detach);
+	} else {
+		// TODO
+	}

 	/*
 	 * Private writable mapping: check memory availability

The next patch now moves the call to vms_complete_munmap_vmas() towards the end of
mmap_region(). This code is also free of the rss counter bug.

commit a4b24bb18dde627792297455befcc465e45be66d
Author: Bert Karwatzki <spasswolf@web.de>
Date:   Thu Jun 20 17:02:08 2024 +0200

    mm: mmap: push back vms_complete_munmap_vmas()

    In order to to debug the rss counter bug we're going to push back
    vms_complete_munmap_vmas() in mmap_region.

    Signed-off-by: Bert Karwatzki <spasswolf@web.de>

diff --git a/mm/mmap.c b/mm/mmap.c
index 0f020c535c83..4fb9dd2e6d6e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2970,9 +2970,9 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		}

 		/* Point of no return */
-		vms_complete_munmap_vmas(&vms, &mas_detach);
 	} else {
-		// TODO
+		vms.end = 0;
+		vms.nr_pages = 0;
 	}

 	/*
@@ -3016,6 +3016,11 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		vma_iter_next_range(&vmi);
 	}

+	if (vms.end) {
+		vms_complete_munmap_vmas(&vms, &mas_detach);
+		vms.end = 0; // avoid double unmap below
+	}
+
 	/* Actually expand, if possible */
 	if (vma &&
 	    !vma_expand(&vmi, vma, merge_start, merge_end, vm_pgoff, next)) {
@@ -3026,7 +3031,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 	if (vma == prev)
 		vma_iter_set(&vmi, addr);
 cannot_expand:
-
+	if (vms.end)
+		vms_complete_munmap_vmas(&vms, &mas_detach);
 	/*
 	 * Determine the object being mapped and call the appropriate
 	 * specific mapper. the address has already been validated, but

The next patch move vms_complete_munmap_vmas() a little further beyond the
call to vma_expand(). This code contain the rss counter bug.

commit 02d6be2410fa503d008f4cc8dcd1518ca56f8793
Author: Bert Karwatzki <spasswolf@web.de>
Date:   Thu Jun 20 20:07:13 2024 +0200

    mm: mmap: push back vms_complete_munmap_vmas()

    This commit actually show the rss counter bug, while the previus does
    not!

    Signed-off-by: Bert Karwatzki <spasswolf@web.de>

diff --git a/mm/mmap.c b/mm/mmap.c
index 4fb9dd2e6d6e..c5f4b4b6fb84 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3016,14 +3016,12 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		vma_iter_next_range(&vmi);
 	}

-	if (vms.end) {
-		vms_complete_munmap_vmas(&vms, &mas_detach);
-		vms.end = 0; // avoid double unmap below
-	}
-
 	/* Actually expand, if possible */
 	if (vma &&
 	    !vma_expand(&vmi, vma, merge_start, merge_end, vm_pgoff, next)) {
+		if (vms.end) {
+			vms_complete_munmap_vmas(&vms, &mas_detach);
+		}
 		khugepaged_enter_vma(vma, vm_flags);
 		goto expanded;
 	}


So there might be some unwanted interaction between vms_complete_munmap_vmas though
I've no yet figured out what exactly is happening. Hope this will be helpful in
solving the problem.

Bert Karwatzki



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] mm: Fix mmap_assert_locked() in follow_pte()
@ 2024-07-12 12:17 Bert Karwatzki
  2024-07-12 12:17 ` commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613 Bert Karwatzki
  0 siblings, 1 reply; 13+ messages in thread
From: Bert Karwatzki @ 2024-07-12 12:17 UTC (permalink / raw)
  To: Pei Li
  Cc: Bert Karwatzki, Andrew Morton, Shuah Khan, David Hildenbrand,
	linux-mm, linux-kernel, linux-next, syzbot+35a4414f6e247f515443,
	syzkaller-bugs, linux-kernel-mentees, senozhatsky

This is causing a deadlock for me, too. Since linux-next-20240712 I observe
a hang when starting the gui. I bisected this to commit a13252049629 and
reverting this commit in linux-next-20240712 fixes the issue for me.
I do not have any log messages to prove the dead lock, though, as I compiled
everything without CONFIG_LOCKDEP.

Bert Karwatzki


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-07-12 12:17 [PATCH] mm: Fix mmap_assert_locked() in follow_pte() Bert Karwatzki
@ 2024-07-12 12:17 ` Bert Karwatzki
  2024-07-12 15:38   ` Liam R. Howlett
  0 siblings, 1 reply; 13+ messages in thread
From: Bert Karwatzki @ 2024-07-12 12:17 UTC (permalink / raw)
  To: Liam R . Howlett
  Cc: Bert Karwatzki, Andrew Morton, Jiri Olsa, linux-mm, linux-kernel,
	linux-next

I did some experiments on the rss counter bug. The next patch is made for linux-next-20240613
with commit 1c29a32ce65f4cd0f1c0f9 reverted. Then I simply inlined the code of do_vmi_unmap()
and do_vmi_align_munmap() into mmap_region(). This version of the code works fine and does not
show the rss counter bug.

diff --git a/mm/mmap.c b/mm/mmap.c
index f95af72ddc9f..0f020c535c83 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -733,7 +733,6 @@ int vma_expand(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	vma_iter_store(vmi, vma);

 	vma_complete(&vp, vmi, vma->vm_mm);
-	validate_mm(vma->vm_mm);
 	return 0;

 nomem:
@@ -2911,6 +2910,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 	struct vm_area_struct *next, *prev, *merge;
 	pgoff_t pglen = len >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	struct vma_munmap_struct vms;
+	struct ma_state mas_detach;
 	unsigned long end = addr + len;
 	unsigned long merge_start = addr, merge_end = end;
 	bool writable_file_mapping = false;
@@ -2933,12 +2934,46 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 			return -ENOMEM;
 	}

-	/* Unmap any existing mapping in the area */
-	error = do_vmi_munmap(&vmi, mm, addr, len, uf, false);
-	if (error == -EPERM)
-		return error;
-	else if (error)
-		return -ENOMEM;
+	/* Find the first overlapping VMA */
+	vma = vma_find(&vmi, end);
+	if (vma) {
+		struct maple_tree mt_detach;
+
+		/*
+		 * Check if memory is sealed before arch_unmap.
+		 * Prevent unmapping a sealed VMA.
+		 * can_modify_mm assumes we have acquired the lock on MM.
+		 */
+		if (unlikely(!can_modify_mm(mm, addr, end))) {
+			return -EPERM;
+		}
+
+		 /* arch_unmap() might do unmaps itself.  */
+		arch_unmap(mm, addr, end);
+
+		mt_init_flags(&mt_detach, vmi.mas.tree->ma_flags & MT_FLAGS_LOCK_MASK);
+		mt_on_stack(mt_detach);
+		mas_init(&mas_detach, &mt_detach, 0);
+
+		init_vma_munmap(&vms, &vmi, vma, addr, end, uf, false);
+		error = vms_gather_munmap_vmas(&vms, &mas_detach);
+		if (error) {
+			validate_mm(mm);
+			return -ENOMEM;
+		}
+
+		vma = NULL;
+		error = vma_iter_clear_gfp(&vmi, addr, end, GFP_KERNEL);
+		if (error) {
+			abort_munmap_vmas(&mas_detach);
+			return -ENOMEM;
+		}
+
+		/* Point of no return */
+		vms_complete_munmap_vmas(&vms, &mas_detach);
+	} else {
+		// TODO
+	}

 	/*
 	 * Private writable mapping: check memory availability

The next patch now moves the call to vms_complete_munmap_vmas() towards the end of
mmap_region(). This code is also free of the rss counter bug.

commit a4b24bb18dde627792297455befcc465e45be66d
Author: Bert Karwatzki <spasswolf@web.de>
Date:   Thu Jun 20 17:02:08 2024 +0200

    mm: mmap: push back vms_complete_munmap_vmas()

    In order to to debug the rss counter bug we're going to push back
    vms_complete_munmap_vmas() in mmap_region.

    Signed-off-by: Bert Karwatzki <spasswolf@web.de>

diff --git a/mm/mmap.c b/mm/mmap.c
index 0f020c535c83..4fb9dd2e6d6e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2970,9 +2970,9 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		}

 		/* Point of no return */
-		vms_complete_munmap_vmas(&vms, &mas_detach);
 	} else {
-		// TODO
+		vms.end = 0;
+		vms.nr_pages = 0;
 	}

 	/*
@@ -3016,6 +3016,11 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		vma_iter_next_range(&vmi);
 	}

+	if (vms.end) {
+		vms_complete_munmap_vmas(&vms, &mas_detach);
+		vms.end = 0; // avoid double unmap below
+	}
+
 	/* Actually expand, if possible */
 	if (vma &&
 	    !vma_expand(&vmi, vma, merge_start, merge_end, vm_pgoff, next)) {
@@ -3026,7 +3031,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 	if (vma == prev)
 		vma_iter_set(&vmi, addr);
 cannot_expand:
-
+	if (vms.end)
+		vms_complete_munmap_vmas(&vms, &mas_detach);
 	/*
 	 * Determine the object being mapped and call the appropriate
 	 * specific mapper. the address has already been validated, but

The next patch move vms_complete_munmap_vmas() a little further beyond the
call to vma_expand(). This code contain the rss counter bug.

commit 02d6be2410fa503d008f4cc8dcd1518ca56f8793
Author: Bert Karwatzki <spasswolf@web.de>
Date:   Thu Jun 20 20:07:13 2024 +0200

    mm: mmap: push back vms_complete_munmap_vmas()

    This commit actually show the rss counter bug, while the previus does
    not!

    Signed-off-by: Bert Karwatzki <spasswolf@web.de>

diff --git a/mm/mmap.c b/mm/mmap.c
index 4fb9dd2e6d6e..c5f4b4b6fb84 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3016,14 +3016,12 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		vma_iter_next_range(&vmi);
 	}

-	if (vms.end) {
-		vms_complete_munmap_vmas(&vms, &mas_detach);
-		vms.end = 0; // avoid double unmap below
-	}
-
 	/* Actually expand, if possible */
 	if (vma &&
 	    !vma_expand(&vmi, vma, merge_start, merge_end, vm_pgoff, next)) {
+		if (vms.end) {
+			vms_complete_munmap_vmas(&vms, &mas_detach);
+		}
 		khugepaged_enter_vma(vma, vm_flags);
 		goto expanded;
 	}


So there might be some unwanted interaction between vms_complete_munmap_vmas though
I've no yet figured out what exactly is happening. Hope this will be helpful in
solving the problem.

Bert Karwatzki



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-07-12 12:17 ` commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613 Bert Karwatzki
@ 2024-07-12 15:38   ` Liam R. Howlett
  2024-07-12 16:26     ` Bert Karwatzki
  0 siblings, 1 reply; 13+ messages in thread
From: Liam R. Howlett @ 2024-07-12 15:38 UTC (permalink / raw)
  To: Bert Karwatzki
  Cc: Andrew Morton, Jiri Olsa, linux-mm, linux-kernel, linux-next

* Bert Karwatzki <spasswolf@web.de> [240712 08:18]:
> I did some experiments on the rss counter bug. The next patch is made for linux-next-20240613
> with commit 1c29a32ce65f4cd0f1c0f9 reverted. Then I simply inlined the code of do_vmi_unmap()
> and do_vmi_align_munmap() into mmap_region(). This version of the code works fine and does not
> show the rss counter bug.

Are you still working with v1 of this patch set?

I root-caused the rss counter issue and seg fault to the fact that next
or prev may be expanded and I was using the new start/end on munmap() in
the completion.  This was fixed in subsequent patches.

I've sent v4 recently, but will have to a v5 to back off the removal of
arch_unmap() for PPC.

...

Thanks,
Liam


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613
  2024-07-12 15:38   ` Liam R. Howlett
@ 2024-07-12 16:26     ` Bert Karwatzki
  0 siblings, 0 replies; 13+ messages in thread
From: Bert Karwatzki @ 2024-07-12 16:26 UTC (permalink / raw)
  To: Liam R. Howlett, Andrew Morton, Jiri Olsa, linux-mm, linux-kernel,
	linux-next

Am Freitag, dem 12.07.2024 um 11:38 -0400 schrieb Liam R. Howlett:
> * Bert Karwatzki <spasswolf@web.de> [240712 08:18]:
> > I did some experiments on the rss counter bug. The next patch is made for linux-next-20240613
> > with commit 1c29a32ce65f4cd0f1c0f9 reverted. Then I simply inlined the code of do_vmi_unmap()
> > and do_vmi_align_munmap() into mmap_region(). This version of the code works fine and does not
> > show the rss counter bug.
>
> Are you still working with v1 of this patch set?
>
> I root-caused the rss counter issue and seg fault to the fact that next
> or prev may be expanded and I was using the new start/end on munmap() in
> the completion.  This was fixed in subsequent patches.
>
> I've sent v4 recently, but will have to a v5 to back off the removal of
> arch_unmap() for PPC.
>
> ...
>
> Thanks,
> Liam

Sorry, that was a mistake when using git send-email while working on another bug
I accidently sent this old mail. I actually have tested the v3 and v4 patchsets
on linux-next up to 2024711 with no error. I havent't tested v4 on linux-
20240712, yet, because next-20240712 has a deadlock issue:
https://lore.kernel.org/all/20240712080414.GA47643@google.com/
(while sending a mail to this issue, I accidently send the old mail, too)

Bert Karwatzki


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-07-12 16:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-12 12:17 [PATCH] mm: Fix mmap_assert_locked() in follow_pte() Bert Karwatzki
2024-07-12 12:17 ` commit 1c29a32ce65f4cd0f1c causes Bad rss-counter state and firefox-esr crash in linux-next-20240613 Bert Karwatzki
2024-07-12 15:38   ` Liam R. Howlett
2024-07-12 16:26     ` Bert Karwatzki
  -- strict thread matches above, loose matches on Subject: below --
2024-06-21 19:32 Bert Karwatzki
2024-06-13 23:40 Bert Karwatzki
2024-06-14  0:03 ` Andrew Morton
2024-06-14  8:26   ` Bert Karwatzki
2024-06-14  8:37   ` Bert Karwatzki
2024-06-14 12:30   ` Liam R. Howlett
2024-06-14 17:21     ` Andrew Morton
2024-06-14  7:54 ` Jiri Olsa
2024-06-14 12:31   ` Liam R. Howlett

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).