Message ID | 1491887293-3815-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 9e1ba4f27f018742a1aa95d11e35106feba08ec1 |
Headers | show |
On Tue, Apr 11, 2017 at 10:38:13AM +0530, Ravi Bangoria wrote: > If we set a kprobe on a 'stdu' instruction on powerpc64, we see a kernel > OOPS: > > [ 1275.165932] Bad kernel stack pointer cd93c840 at c000000000009868 > [ 1275.166378] Oops: Bad kernel stack pointer, sig: 6 [#1] > ... > GPR00: c000001fcd93cb30 00000000cd93c840 c0000000015c5e00 00000000cd93c840 > ... > [ 1275.178305] NIP [c000000000009868] resume_kernel+0x2c/0x58 > [ 1275.178594] LR [c000000000006208] program_check_common+0x108/0x180 > > Basically, on 64 bit system, when user probes on 'stdu' instruction, > kernel does not emulate actual store in emulate_step itself because it > may corrupt exception frame. So kernel does actual store operation in > exception return code i.e. resume_kernel(). > > resume_kernel() loads the saved stack pointer from memory using lwz, > effectively loading a corrupt (32bit) address, causing the kernel crash. > > Fix this by loading the 64bit value instead. > > Fixes: be96f63375a1 ("powerpc: Split out instruction analysis part of emulate_step()") > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> > Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> Reviewed-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
On Tue, 2017-04-11 at 10:38 +0530, Ravi Bangoria wrote: > If we set a kprobe on a 'stdu' instruction on powerpc64, we see a kernel > OOPS: > > [ 1275.165932] Bad kernel stack pointer cd93c840 at c000000000009868 > [ 1275.166378] Oops: Bad kernel stack pointer, sig: 6 [#1] > ... > GPR00: c000001fcd93cb30 00000000cd93c840 c0000000015c5e00 00000000cd93c840 > ... > [ 1275.178305] NIP [c000000000009868] resume_kernel+0x2c/0x58 > [ 1275.178594] LR [c000000000006208] program_check_common+0x108/0x180 > > Basically, on 64 bit system, when user probes on 'stdu' instruction, > kernel does not emulate actual store in emulate_step itself because it > may corrupt exception frame. So kernel does actual store operation in > exception return code i.e. resume_kernel(). > > resume_kernel() loads the saved stack pointer from memory using lwz, > effectively loading a corrupt (32bit) address, causing the kernel crash. > > Fix this by loading the 64bit value instead. > > Fixes: be96f63375a1 ("powerpc: Split out instruction analysis part of emulate_step()") > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> > Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- The patch looks correct to me from the description and code. I have not validated that the write to GPR1(r1) via store of r8 to 0(r5) is indeed correct. I would assume r8 should contain regs->gpr[r1] with the updated ea that is written down to the GPR1(r1) which will be what we restore when we return from the exception. The conversion of lwz to ld indeed looks correct Balbir Singh.
Thanks Balbir for the review, On Tuesday 11 April 2017 02:25 PM, Balbir Singh wrote: > On Tue, 2017-04-11 at 10:38 +0530, Ravi Bangoria wrote: >> If we set a kprobe on a 'stdu' instruction on powerpc64, we see a kernel >> OOPS: >> >> [ 1275.165932] Bad kernel stack pointer cd93c840 at c000000000009868 >> [ 1275.166378] Oops: Bad kernel stack pointer, sig: 6 [#1] >> ... >> GPR00: c000001fcd93cb30 00000000cd93c840 c0000000015c5e00 00000000cd93c840 >> ... >> [ 1275.178305] NIP [c000000000009868] resume_kernel+0x2c/0x58 >> [ 1275.178594] LR [c000000000006208] program_check_common+0x108/0x180 >> >> Basically, on 64 bit system, when user probes on 'stdu' instruction, >> kernel does not emulate actual store in emulate_step itself because it >> may corrupt exception frame. So kernel does actual store operation in >> exception return code i.e. resume_kernel(). >> >> resume_kernel() loads the saved stack pointer from memory using lwz, >> effectively loading a corrupt (32bit) address, causing the kernel crash. >> >> Fix this by loading the 64bit value instead. >> >> Fixes: be96f63375a1 ("powerpc: Split out instruction analysis part of emulate_step()") >> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> >> Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> >> --- > The patch looks correct to me from the description and code. I have not > validated that the write to GPR1(r1) via store of r8 to 0(r5) is indeed correct. > I would assume r8 should contain regs->gpr[r1] with the updated ea that > is written down to the GPR1(r1) which will be what we restore when we return > from the exception. emulate_step() updates regs->gpr[r1] with the new value. So, regs->gpr[r1] and GPR(r1) both are same at resume_kernel. At resume_kernel, r1 points to the exception frame. Address of frame preceding exception frame gets loaded in r8 with: addi r8,r1,INT_FRAME_SIZE Let me know if you need more details. Ravi
On Tue, 2017-04-11 at 05:08:13 UTC, Ravi Bangoria wrote: > If we set a kprobe on a 'stdu' instruction on powerpc64, we see a kernel > OOPS: > > [ 1275.165932] Bad kernel stack pointer cd93c840 at c000000000009868 > [ 1275.166378] Oops: Bad kernel stack pointer, sig: 6 [#1] > ... > GPR00: c000001fcd93cb30 00000000cd93c840 c0000000015c5e00 00000000cd93c840 > ... > [ 1275.178305] NIP [c000000000009868] resume_kernel+0x2c/0x58 > [ 1275.178594] LR [c000000000006208] program_check_common+0x108/0x180 > > Basically, on 64 bit system, when user probes on 'stdu' instruction, > kernel does not emulate actual store in emulate_step itself because it > may corrupt exception frame. So kernel does actual store operation in > exception return code i.e. resume_kernel(). > > resume_kernel() loads the saved stack pointer from memory using lwz, > effectively loading a corrupt (32bit) address, causing the kernel crash. > > Fix this by loading the 64bit value instead. > > Fixes: be96f63375a1 ("powerpc: Split out instruction analysis part of emulate_step()") > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> > Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > Reviewed-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/9e1ba4f27f018742a1aa95d11e3510 cheers
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index 6432d4b..767ef6d 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -689,7 +689,7 @@ resume_kernel: addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ - lwz r3,GPR1(r1) + ld r3,GPR1(r1) subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ mr r4,r1 /* src: current exception frame */ mr r1,r3 /* Reroute the trampoline frame to r1 */ @@ -703,8 +703,8 @@ resume_kernel: addi r6,r6,8 bdnz 2b - /* Do real store operation to complete stwu */ - lwz r5,GPR1(r1) + /* Do real store operation to complete stdu */ + ld r5,GPR1(r1) std r8,0(r5) /* Clear _TIF_EMULATE_STACK_STORE flag */