From: Michael Ellerman <mpe@ellerman.id.au>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
Date: Fri, 25 Jun 2021 14:48:50 +1000 [thread overview]
Message-ID: <871r8q1rd9.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <b44b66f4-3e46-425b-3c8e-d5a7a180494a@csgroup.eu>
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
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] powerpc/kprobes: Fix Oops by passing ppc_inst as a pointer to emulate_step() on ppc32
Date: Fri, 25 Jun 2021 14:48:50 +1000 [thread overview]
Message-ID: <871r8q1rd9.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <b44b66f4-3e46-425b-3c8e-d5a7a180494a@csgroup.eu>
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
next prev parent reply other threads:[~2021-06-25 4:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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-23 5:23 ` Christophe Leroy
2021-06-24 10:59 ` Naveen N. Rao
2021-06-24 10:59 ` Naveen N. Rao
2021-06-24 11:09 ` Christophe Leroy
2021-06-24 11:09 ` Christophe Leroy
2021-06-25 4:48 ` Michael Ellerman [this message]
2021-06-25 4:48 ` Michael Ellerman
2021-06-26 10:37 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871r8q1rd9.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman.id.au \
--cc=benh@kernel.crashing.org \
--cc=christophe.leroy@csgroup.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=naveen.n.rao@linux.ibm.com \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.