On Thu, May 12, 2016 at 03:51:31PM +0200, Borislav Petkov wrote:
> On Thu, May 12, 2016 at 06:34:29AM -0700, Guenter Roeck wrote:
> > Borislav,
> >
> > your patch 'locking/rwsem, x86: Clean up ____down_write()' causes various
> > crashes in x86 qemu tests.
>
> Thanks for the report, let me take a look.
>
> @Ingo: can you please back this one out of the lineup for the merge
> window until I've sorted out the issue?
Ok, I was able to reproduce:
BUG: unable to handle kernel NULL pointer dereference at 00000015
IP: [<c185e094>] down_write+0x24/0x30
*pde = 00000000
Oops: 0002 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Tainted: G S W 4.6.0-rc7-next-20160511-yocto-standard #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
task: f4d00000 ti: f4d08000 task.ti: f4d08000
EIP: 0060:[<c185e094>] EFLAGS: 00210282 CPU: 0
EIP is at down_write+0x24/0x30
EAX: f4d00000 EBX: f4f6d600 ECX: ffff0001 EDX: 00000001
ESI: 00000168 EDI: c1c2eb68 EBP: f4d09ef4 ESP: f4d09eec
DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
CR0: 80050033 CR2: 00000015 CR3: 01ccb000 CR4: 000406d0
We fault here:
c185e070 <down_write>:
c185e070: 55 push %ebp
c185e071: 89 e5 mov %esp,%ebp
c185e073: e8 20 2b 00 00 call c1860b98 <mcount>
c185e078: b9 01 00 ff ff mov $0xffff0001,%ecx
c185e07d: 89 c2 mov %eax,%edx
c185e07f: f0 0f c1 08 lock xadd %ecx,(%eax)
c185e083: 66 85 c9 test %cx,%cx
c185e086: 74 05 je c185e08d <down_write+0x1d>
c185e088: e8 f7 31 b7 ff call c13d1284 <call_rwsem_down_write_failed>
c185e08d: 64 a1 48 59 cb c1 mov %fs:0xc1cb5948,%eax
c185e093: 5d pop %ebp
c185e094: 89 42 14 mov %eax,0x14(%edx) <--- HERE
c185e097: c3 ret
c185e098: 90 nop
c185e099: 8d b4 26 00 00 00 00 lea 0x0(%esi,%eiz,1),%esi
and %edx is 1 (+ 0x14 gives the 00000015 deref addr).
But edx should contain sem. The code does:
.loc 1 47 0
movl %eax, %edx # sem, sem
lock; xadd %ecx,(%eax) # tmp91, sem
call call_rwsem_down_write_failed
mov %eax,0x14(%edx)
and if something in that call clobbers %edx, boom! Now I need to think
about how to make gcc reload sem after
LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
for
rwsem_set_owner(sem);
Btw, the hunk below seems to fix it. And the comment above those
{save,restore}_common_regs talk about "Save the C-clobbered registers
(%eax, %edx and %ecx)" but the only reg we're stashing is ecx.
Why aren't we stashing edx too?
Ingo, Peter?
---
diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S
index a37462a23546..02240807e97a 100644
--- a/arch/x86/lib/rwsem.S
+++ b/arch/x86/lib/rwsem.S
@@ -33,10 +33,12 @@
* value or just clobbered..
*/
-#define save_common_regs \
- pushl %ecx
+#define save_common_regs \
+ pushl %ecx; \
+ pushl %edx
-#define restore_common_regs \
+#define restore_common_regs \
+ popl %edx; \
popl %ecx
/* Avoid uglifying the argument copying x86-64 needs to do. */
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
