Shared memory shmat/dt not working well in

PATCH Petr Vandrovec 4 participants 5 messages Oct 1, 2002

Subsystems (2)

Memory management Memory mapping

lore

Share & tools

lore

Petr Vandrovec

author

2002-10-01 15:58 UTC #0 lore raw

Quoted 3 lines · Hugh Dickins

Exemplary bug report! Many thanks for taking so much trouble to reproduce the problem. Patch below (against 2.5.39) should fix it:
I'll send Linus and Andrew when I can get hold of a 2.5.40 tree.

You are my hero! It fixed dnetc client failing with SIGSEGV after call to sys_mprotect().

Unfortunately it did not fixed another problem I have with sys_mprotect. If I start X, system stops to do anything useful. After SAK I could do remote connect and running 'w' and 'ps axf' moved them to 'D' state. Clean sysrq-u,s,b was possible. 2.5.40 kernel plus Hugh's patch, 100% reproducible with 2.5.40 SMP (non-preempt) kernel running on UP here...
Thanks,
Petr Vandrovec

Oct  1 17:47:35 vana kernel: SysRq : SAK
Oct  1 17:47:35 vana kernel: SAK: killed process 1482 (XFree86): fd#3 opened to the tty
...
Oct  1 17:47:55 vana kernel: XFree86       D DADE7A60     0  1482   1481                     (NOTLB)
Oct  1 17:47:55 vana kernel: Call Trace:
Oct  1 17:47:55 vana kernel:  [<c01167df>]schedule+0x3bb/0x488
Oct  1 17:47:55 vana kernel:  [<c019bdd5>]rwsem_down_read_failed+0x165/0x188
Oct  1 17:47:55 vana kernel:  [<c0114adb>].text.lock.fault+0x7/0x6c
Oct  1 17:47:55 vana kernel:  [<c01146b0>]do_page_fault+0x0/0x424
Oct  1 17:47:55 vana kernel:  [<c01323a2>]do_generic_file_read+0xee/0x35c
Oct  1 17:47:55 vana kernel:  [<c0132602>]do_generic_file_read+0x34e/0x35c
Oct  1 17:47:55 vana kernel:  [<c01328cb>]__generic_file_aio_read+0x187/0x1a0
Oct  1 17:47:55 vana kernel:  [<c0132610>]file_read_actor+0x0/0x134
Oct  1 17:47:55 vana kernel:  [<c01372bc>]free_block+0x168/0x250
Oct  1 17:47:55 vana kernel:  [<c0107ec9>]error_code+0x2d/0x38
Oct  1 17:47:55 vana kernel:  [<c011216f>]flush_tlb_mm+0x1b/0x70
Oct  1 17:47:55 vana kernel:  [<c0133ed1>]change_protection+0x1a1/0x1dc
Oct  1 17:47:55 vana kernel:  [<c01341ed>]mprotect_fixup+0x16d/0x188
Oct  1 17:47:55 vana kernel:  [<c0134392>]sys_mprotect+0x18a/0x2c4
Oct  1 17:47:55 vana kernel:  [<c0107467>]syscall_call+0x7/0xb
...
Oct  1 17:47:55 vana kernel: w             D DADE7A60     0  1563   1558                     (NOTLB)
Oct  1 17:47:55 vana kernel: Call Trace:
Oct  1 17:47:55 vana kernel:  [<c015be79>]dput+0x19/0x1d4
Oct  1 17:47:55 vana kernel:  [<c019bdd5>]rwsem_down_read_failed+0x165/0x188
Oct  1 17:47:55 vana kernel:  [<c016fecf>].text.lock.array+0x6d/0x10e
Oct  1 17:47:55 vana kernel:  [<c013b238>]__get_free_pages+0x28/0x60
Oct  1 17:47:55 vana kernel:  [<c016c807>]proc_info_read+0x4f/0x108
Oct  1 17:47:55 vana kernel:  [<c0146580>]vfs_read+0xb4/0x134
Oct  1 17:47:55 vana kernel:  [<c014676a>]sys_read+0x2a/0x3c
Oct  1 17:47:55 vana kernel:  [<c0107467>]syscall_call+0x7/0xb
...
Oct  1 17:47:55 vana kernel: ps            D DADE7A60     0  1583   1582                     (NOTLB)
Oct  1 17:47:55 vana kernel: Call Trace:
Oct  1 17:47:55 vana kernel:  [<c015be79>]dput+0x19/0x1d4
Oct  1 17:47:55 vana kernel:  [<c019bdd5>]rwsem_down_read_failed+0x165/0x188
Oct  1 17:47:55 vana kernel:  [<c016fecf>].text.lock.array+0x6d/0x10e
Oct  1 17:47:55 vana kernel:  [<c013b238>]__get_free_pages+0x28/0x60
Oct  1 17:47:55 vana kernel:  [<c016c807>]proc_info_read+0x4f/0x108
Oct  1 17:47:55 vana kernel:  [<c0146580>]vfs_read+0xb4/0x134
Oct  1 17:47:55 vana kernel:  [<c014676a>]sys_read+0x2a/0x3c
Oct  1 17:47:55 vana kernel:  [<c0107467>]syscall_call+0x7/0xb

Hugh Dickins

2002-10-01 17:10 UTC #1 lore raw

Quoted 2 lines · Petr Vandrovec

You are my hero!

Aww, shucks! I guess I'd better have a go at your next problem...

Quoted 8 lines

Unfortunately it did not fixed another problem I have with sys_mprotect. If I start X, system stops to do anything useful. After SAK I could do remote connect and running 'w' and 'ps axf' moved them to 'D' state.
Oct  1 17:47:55 vana kernel:  [<c0107ec9>]error_code+0x2d/0x38
Oct  1 17:47:55 vana kernel:  [<c011216f>]flush_tlb_mm+0x1b/0x70
Oct  1 17:47:55 vana kernel:  [<c0133ed1>]change_protection+0x1a1/0x1dc
Oct  1 17:47:55 vana kernel:  [<c01341ed>]mprotect_fixup+0x16d/0x188

Looks to me like flush_tlb_mm is faulting on the vma->vm_mm given it. And looks to me like mprotect_fixup, in the merge case, may be passing an already freed vma to change_protection. I'm not as confident about this patch as the earlier one, but I believe it's correct: please give it a try, and maybe Christoph will confirm or deny it.

Hugh

mm/mprotect.c +3 -1

@@ -186,8 +186,10 @@
 		/*
 		 * Try to merge with the previous vma.
 		 */
-		if (mprotect_attempt_merge(vma, *pprev, end, newflags))
+		if (mprotect_attempt_merge(vma, *pprev, end, newflags)) {
+			vma = *pprev;
 			goto success;
+		}
 	} else {
 		error = split_vma(mm, vma, start, 1);
 		if (error)

Petr Vandrovec

author

2002-10-01 17:50 UTC #2 lore raw

Quoted 11 lines · Hugh Dickins

Quoted 2 lines · Petr Vandrovec
You are my hero!
Aww, shucks! I guess I'd better have a go at your next problem...
Looks to me like flush_tlb_mm is faulting on the vma->vm_mm given it. And looks to me like mprotect_fixup, in the merge case, may be passing an already freed vma to change_protection. I'm not as confident about this patch as the earlier one, but I believe it's correct: please give it a try, and maybe Christoph will confirm or deny it.

Thanks. It fixed my problem (again ;-) ). Maybe I should already asked yesterday instead of starring at mprotect code.
Many thanks,
Petr Vandrovec [email protected]

Quoted 22 lines

Hugh
mm/mprotect.c +3 -2
@@ -186,8 +186,10 @@
 		/*
 		 * Try to merge with the previous vma.
 		 */
-		if (mprotect_attempt_merge(vma, *pprev, end, newflags))
+		if (mprotect_attempt_merge(vma, *pprev, end, newflags)) {
+			vma = *pprev;
 			goto success;
+		}
 	} else {
 		error = split_vma(mm, vma, start, 1);
 		if (error)
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Christoph Hellwig

2002-10-01 18:56 UTC #3 lore raw

In reply to Hugh Dickins

Quoted 4 lines · Hugh Dickins

And looks to me like mprotect_fixup, in the merge case, may be passing an already freed vma to change_protection. I'm not as confident about this patch as the earlier one, but I believe it's correct: please give it a try, and maybe Christoph will confirm or deny it.

Looks good. My intial patch didn't do the the change_protection at all, and it looks like the fix someone (akpm?) submitted wasn't exactly correct.

Andrew Morton

2002-10-01 19:10 UTC #4 lore raw

In reply to Hugh Dickins

Quoted 6 lines · Hugh Dickins

...
mm/mprotect.c Tue Oct 1 18:00:31 2002 renamed from mm/mprotect.c Fri Sep 27 23:56:45 2002 +0 -0
@@ -186,8 +186,10 @@
                /*

mm/mprotect.c Tue Oct 1 18:00:31 2002 line 195

Quoted 10 lines

mm/mprotect.c Tue Oct 1 18:00:31 2002 renamed from mm/mprotect.c Fri Sep 27 23:56:45 2002 +3 -1

                 * Try to merge with the previous vma.
                 */
-               if (mprotect_attempt_merge(vma, *pprev, end, newflags))
+               if (mprotect_attempt_merge(vma, *pprev, end, newflags)) {
+                       vma = *pprev;
                        goto success;
+               }
        } else {
                error = split_vma(mm, vma, start, 1);
                if (error)

Got that, thanks.

I think that's enough "cleanups" for 2.5, guys. Let's concentrate on missing must-have functionality, bugs and tuning from now.