linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
@ 2021-06-23  5:23 Christophe Leroy
  2021-06-24 10:59 ` Naveen N. Rao
  2021-06-26 10:37 ` Michael Ellerman
  0 siblings, 2 replies; 5+ messages in thread
From: Christophe Leroy @ 2021-06-23  5:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

From: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Trying to use a kprobe on ppc32 results in the below splat:
    BUG: Unable to handle kernel data access on read at 0x7c0802a6
    Faulting instruction address: 0xc002e9f0
    Oops: Kernel access of bad area, sig: 11 [#1]
    BE PAGE_SIZE=4K PowerPC 44x Platform
    Modules linked in:
    CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7
    NIP:  c002e9f0 LR: c0011858 CTR: 00008a47
    REGS: c292fd50 TRAP: 0300   Not tainted  (5.13.0-rc1-01824-g3a81c0495fdb)
    MSR:  00009000 <EE,ME>  CR: 24002002  XER: 20000000
    DEAR: 7c0802a6 ESR: 00000000
    <snip>
    NIP [c002e9f0] emulate_step+0x28/0x324
    LR [c0011858] optinsn_slot+0x128/0x10000
    Call Trace:
     opt_pre_handler+0x7c/0xb4 (unreliable)
     optinsn_slot+0x128/0x10000
     ret_from_syscall+0x0/0x28

The offending instruction is:
    81 24 00 00     lwz     r9,0(r4)

Here, we are trying to load the second argument to emulate_step():
struct ppc_inst, which is the instruction to be emulated. On ppc64,
structures are passed in registers when passed by value. However, per
the ppc32 ABI, structures are always passed to functions as pointers.
This isn't being adhered to when setting up the call to emulate_step()
in the optprobe trampoline. Fix the same.

Fixes: eacf4c0202654a ("powerpc: Enable OPTPROBES on PPC32")
Cc: stable@vger.kernel.org
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
v2: Rebased on powerpc/merge 7f030e9d57b8
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/optprobes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 2b8fe40069ad..53facb4b377f 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -228,8 +228,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	/*
 	 * 3. load instruction to be emulated into relevant register, and
 	 */
-	temp = ppc_inst_read(p->ainsn.insn);
-	patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
+	if (IS_ENABLED(CONFIG_PPC64)) {
+		temp = ppc_inst_read(p->ainsn.insn);
+		patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
+	} else {
+		patch_imm_load_insns((unsigned long)p->ainsn.insn, 4, buff + TMPL_INSN_IDX);
+	}
 
 	/*
 	 * 4. branch back from trampoline
-- 
2.25.0


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

* Re: [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
  2021-06-23  5:23 [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32 Christophe Leroy
@ 2021-06-24 10:59 ` Naveen N. Rao
  2021-06-24 11:09   ` Christophe Leroy
  2021-06-26 10:37 ` Michael Ellerman
  1 sibling, 1 reply; 5+ messages in thread
From: Naveen N. Rao @ 2021-06-24 10:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
	Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy wrote:
> From: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> 
> Trying to use a kprobe on ppc32 results in the below splat:
>     BUG: Unable to handle kernel data access on read at 0x7c0802a6
>     Faulting instruction address: 0xc002e9f0
>     Oops: Kernel access of bad area, sig: 11 [#1]
>     BE PAGE_SIZE=4K PowerPC 44x Platform
>     Modules linked in:
>     CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7
>     NIP:  c002e9f0 LR: c0011858 CTR: 00008a47
>     REGS: c292fd50 TRAP: 0300   Not tainted  (5.13.0-rc1-01824-g3a81c0495fdb)
>     MSR:  00009000 <EE,ME>  CR: 24002002  XER: 20000000
>     DEAR: 7c0802a6 ESR: 00000000
>     <snip>
>     NIP [c002e9f0] emulate_step+0x28/0x324
>     LR [c0011858] optinsn_slot+0x128/0x10000
>     Call Trace:
>      opt_pre_handler+0x7c/0xb4 (unreliable)
>      optinsn_slot+0x128/0x10000
>      ret_from_syscall+0x0/0x28
> 
> The offending instruction is:
>     81 24 00 00     lwz     r9,0(r4)
> 
> Here, we are trying to load the second argument to emulate_step():
> struct ppc_inst, which is the instruction to be emulated. On ppc64,
> structures are passed in registers when passed by value. However, per
> the ppc32 ABI, structures are always passed to functions as pointers.
> This isn't being adhered to when setting up the call to emulate_step()
> in the optprobe trampoline. Fix the same.
> 
> Fixes: eacf4c0202654a ("powerpc: Enable OPTPROBES on PPC32")
> Cc: stable@vger.kernel.org
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> v2: Rebased on powerpc/merge 7f030e9d57b8
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Thanks for rebasing this!

I think git am drops everything after three dashes, so applying this 
patch will drop your SOB. The recommended style (*) for adding a 
changelog is to include it within [] before the second SOB.


- Naveen

(*) https://www.kernel.org/doc/html/latest/maintainer/modifying-patches.html


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

* Re: [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
  2021-06-24 10:59 ` Naveen N. Rao
@ 2021-06-24 11:09   ` Christophe Leroy
  2021-06-25  4:48     ` Michael Ellerman
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2021-06-24 11:09 UTC (permalink / raw)
  To: Naveen N. Rao, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev



Le 24/06/2021 à 12:59, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> From: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>
>> Trying to use a kprobe on ppc32 results in the below splat:
>>     BUG: Unable to handle kernel data access on read at 0x7c0802a6
>>     Faulting instruction address: 0xc002e9f0
>>     Oops: Kernel access of bad area, sig: 11 [#1]
>>     BE PAGE_SIZE=4K PowerPC 44x Platform
>>     Modules linked in:
>>     CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7
>>     NIP:  c002e9f0 LR: c0011858 CTR: 00008a47
>>     REGS: c292fd50 TRAP: 0300   Not tainted  (5.13.0-rc1-01824-g3a81c0495fdb)
>>     MSR:  00009000 <EE,ME>  CR: 24002002  XER: 20000000
>>     DEAR: 7c0802a6 ESR: 00000000
>>     <snip>
>>     NIP [c002e9f0] emulate_step+0x28/0x324
>>     LR [c0011858] optinsn_slot+0x128/0x10000
>>     Call Trace:
>>      opt_pre_handler+0x7c/0xb4 (unreliable)
>>      optinsn_slot+0x128/0x10000
>>      ret_from_syscall+0x0/0x28
>>
>> The offending instruction is:
>>     81 24 00 00     lwz     r9,0(r4)
>>
>> Here, we are trying to load the second argument to emulate_step():
>> struct ppc_inst, which is the instruction to be emulated. On ppc64,
>> structures are passed in registers when passed by value. However, per
>> the ppc32 ABI, structures are always passed to functions as pointers.
>> This isn't being adhered to when setting up the call to emulate_step()
>> in the optprobe trampoline. Fix the same.
>>
>> Fixes: eacf4c0202654a ("powerpc: Enable OPTPROBES on PPC32")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>> v2: Rebased on powerpc/merge 7f030e9d57b8
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> Thanks for rebasing this!
> 
> I think git am drops everything after three dashes, so applying this patch will drop your SOB. The 
> recommended style (*) for adding a changelog is to include it within [] before the second SOB.
> 

Yes, I saw that after sending the mail. Usually I add a signed-off-by with 'git --amend -s' when I 
add the history, so it goes before the '---' I'm adding.

This time I must have forgotten the '-s' so it was added by the 'git format-patch -s' which is in my 
submit script, and so it was added at the end.

It's not really a big deal, up to Michael to either move it at the right place or discard it, I 
don't really mind. Do the easiest for you.

Thanks
Christophe

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

* Re: [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
  2021-06-24 11:09   ` Christophe Leroy
@ 2021-06-25  4:48     ` Michael Ellerman
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2021-06-25  4:48 UTC (permalink / raw)
  To: Christophe Leroy, Naveen N. Rao, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 24/06/2021 à 12:59, Naveen N. Rao a écrit :
>> Christophe Leroy wrote:
>>> From: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>>
>>> Trying to use a kprobe on ppc32 results in the below splat:
>>>     BUG: Unable to handle kernel data access on read at 0x7c0802a6
>>>     Faulting instruction address: 0xc002e9f0
>>>     Oops: Kernel access of bad area, sig: 11 [#1]
>>>     BE PAGE_SIZE=4K PowerPC 44x Platform
>>>     Modules linked in:
>>>     CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7
>>>     NIP:  c002e9f0 LR: c0011858 CTR: 00008a47
>>>     REGS: c292fd50 TRAP: 0300   Not tainted  (5.13.0-rc1-01824-g3a81c0495fdb)
>>>     MSR:  00009000 <EE,ME>  CR: 24002002  XER: 20000000
>>>     DEAR: 7c0802a6 ESR: 00000000
>>>     <snip>
>>>     NIP [c002e9f0] emulate_step+0x28/0x324
>>>     LR [c0011858] optinsn_slot+0x128/0x10000
>>>     Call Trace:
>>>      opt_pre_handler+0x7c/0xb4 (unreliable)
>>>      optinsn_slot+0x128/0x10000
>>>      ret_from_syscall+0x0/0x28
>>>
>>> The offending instruction is:
>>>     81 24 00 00     lwz     r9,0(r4)
>>>
>>> Here, we are trying to load the second argument to emulate_step():
>>> struct ppc_inst, which is the instruction to be emulated. On ppc64,
>>> structures are passed in registers when passed by value. However, per
>>> the ppc32 ABI, structures are always passed to functions as pointers.
>>> This isn't being adhered to when setting up the call to emulate_step()
>>> in the optprobe trampoline. Fix the same.
>>>
>>> Fixes: eacf4c0202654a ("powerpc: Enable OPTPROBES on PPC32")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>> ---
>>> v2: Rebased on powerpc/merge 7f030e9d57b8
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> 
>> Thanks for rebasing this!
>> 
>> I think git am drops everything after three dashes, so applying this patch will drop your SOB. The 
>> recommended style (*) for adding a changelog is to include it within [] before the second SOB.
>
> Yes, I saw that after sending the mail. Usually I add a signed-off-by with 'git --amend -s' when I 
> add the history, so it goes before the '---' I'm adding.
>
> This time I must have forgotten the '-s' so it was added by the 'git format-patch -s' which is in my 
> submit script, and so it was added at the end.
>
> It's not really a big deal, up to Michael to either move it at the right place or discard it, I 
> don't really mind. Do the easiest for you.

I just added Christophe's SoB.

cheers

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

* Re: [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
  2021-06-23  5:23 [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32 Christophe Leroy
  2021-06-24 10:59 ` Naveen N. Rao
@ 2021-06-26 10:37 ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2021-06-26 10:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
	Christophe Leroy
  Cc: linuxppc-dev, linux-kernel

On Wed, 23 Jun 2021 05:23:30 +0000 (UTC), Christophe Leroy wrote:
> Trying to use a kprobe on ppc32 results in the below splat:
>     BUG: Unable to handle kernel data access on read at 0x7c0802a6
>     Faulting instruction address: 0xc002e9f0
>     Oops: Kernel access of bad area, sig: 11 [#1]
>     BE PAGE_SIZE=4K PowerPC 44x Platform
>     Modules linked in:
>     CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7
>     NIP:  c002e9f0 LR: c0011858 CTR: 00008a47
>     REGS: c292fd50 TRAP: 0300   Not tainted  (5.13.0-rc1-01824-g3a81c0495fdb)
>     MSR:  00009000 <EE,ME>  CR: 24002002  XER: 20000000
>     DEAR: 7c0802a6 ESR: 00000000
>     <snip>
>     NIP [c002e9f0] emulate_step+0x28/0x324
>     LR [c0011858] optinsn_slot+0x128/0x10000
>     Call Trace:
>      opt_pre_handler+0x7c/0xb4 (unreliable)
>      optinsn_slot+0x128/0x10000
>      ret_from_syscall+0x0/0x28
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
      https://git.kernel.org/powerpc/c/511eea5e2ccdfdbf3d626bde0314e551f247dd18

cheers

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

end of thread, other threads:[~2021-06-26 10:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23  5:23 [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32 Christophe Leroy
2021-06-24 10:59 ` Naveen N. Rao
2021-06-24 11:09   ` Christophe Leroy
2021-06-25  4:48     ` Michael Ellerman
2021-06-26 10:37 ` Michael Ellerman

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