Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Hongyan Xia <hongyan.xia@transsion.com>
To: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: Pu Hu <hupu@transsion.com>, Jiazi Li <jiazi.li@transsion.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"naveen@kernel.org" <naveen@kernel.org>,
	"yang@os.amperecomputing.com" <yang@os.amperecomputing.com>,
	"will@kernel.org" <will@kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-trace-kernel@vger.kernel.org"
	<linux-trace-kernel@vger.kernel.org>
Subject: Re: [RFC 1/3] arm64: kprobes: Do not handle non-XOL faults as kprobe faults
Date: Wed, 8 Jul 2026 12:12:43 +0000	[thread overview]
Message-ID: <89a5b9a3-0bdf-4781-bfc3-9f6fe88e070d@transsion.com> (raw)
In-Reply-To: <20260708205947.4af54d7638f115e915e9c7ee@kernel.org>

On 7/8/2026 7:59 PM, Masami Hiramatsu wrote:
> On Wed, 8 Jul 2026 08:55:37 +0000
> Hongyan Xia <hongyan.xia@transsion.com> wrote:
>
>> On 7/8/2026 3:42 PM, Masami Hiramatsu wrote:
>>> On Wed, 8 Jul 2026 05:57:24 +0000
>>> Hongyan Xia <hongyan.xia@transsion.com> wrote:
>>>
>>>> Hi Masami,
>>>>
>>>> On 7/8/2026 8:46 AM, Masami Hiramatsu wrote:
>>>>> On Mon, 6 Jul 2026 08:36:48 +0000
>>>>> Pu Hu <hupu@transsion.com> wrote:
>>>>>
>>>>>> From: Pu Hu <hupu@transsion.com>
>>>>>>
>>>>>> kprobe_fault_handler() handles faults taken while kprobes is in
>>>>>> KPROBE_HIT_SS or KPROBE_REENTER state as faults caused by the
>>>>>> single-stepped instruction.
>>>>>>
>>>>>> That assumption is not always true. While a kprobe is preparing or
>>>>>> executing the out-of-line single-step instruction, other code may run
>>>>>> in that window. For example, perf or trace code can be invoked from the
>>>>>> debug exception path and may take a fault of its own. In that case the
>>>>>> fault did not happen on the kprobe XOL instruction, but the kprobe fault
>>>>>> handler may still try to recover it as a kprobe single-step fault.
>>>>>>
>>>>>> This can corrupt the exception recovery flow and leave the real fault to
>>>>>> be handled with a wrong PC. A typical reproducer is running simpleperf
>>>>>> with preemptirq tracepoints and dwarf callchains while a kprobe is
>>>>>> installed on a frequently executed kernel function.
>>>>>>
>>>>>> Fix this by handling faults in KPROBE_HIT_SS/KPROBE_REENTER only when
>>>>>> the faulting PC points at the current kprobe's XOL instruction. Faults
>>>>>> from any other PC are left to the normal fault handling path.
>>>>>>
>>>>>> This follows the same idea as the x86 fix in commit 6381c24cd6d5
>>>>>> ("kprobes/x86: Fix page-fault handling logic").
>>>>>>
>>>>>> Signed-off-by: Pu Hu <hupu@transsion.com>
>>>>>> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
>>>>>> ---
>>>>>>     arch/arm64/kernel/probes/kprobes.c | 14 ++++++++++++++
>>>>>>     1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
>>>>>> index 43a0361a8bf0..e4d2852ce2fb 100644
>>>>>> --- a/arch/arm64/kernel/probes/kprobes.c
>>>>>> +++ b/arch/arm64/kernel/probes/kprobes.c
>>>>>> @@ -285,6 +285,20 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
>>>>>>          switch (kcb->kprobe_status) {
>>>>>>          case KPROBE_HIT_SS:
>>>>>>          case KPROBE_REENTER:
>>>>>> +             /*
>>>>>> +              * A fault taken while a kprobe is single-stepping is not
>>>>>> +              * necessarily caused by the instruction in the XOL slot. For
>>>>>> +              * example, tracing or perf code running in this window may take
>>>>>> +              * an unrelated fault.
>>>>>> +              *
>>>>>> +              * Handle the fault here only when the faulting PC is the XOL
>>>>>> +              * instruction of the current kprobe. Otherwise let the normal
>>>>>> +              * fault handling path deal with it.
>>>>>> +              */
>>>>>> +             if (cur->ainsn.xol_insn &&
>>>>>> +                     instruction_pointer(regs) != (unsigned long)cur->ainsn.xol_insn)
>>>>>> +                     break;
>>>>>
>>>>> Can you check Sashiko's comments[1]?
>>>>>
>>>>> [1] https://sashiko.dev/#/patchset/20260706083636.159883-1-hupu%40transsion.com?part=1
>>>>>
>>>>> It seems that it complains about simulated kprobe's case.
>>>>> In that case, cur->ainsn.xol_insn == NULL. The simulation should be done
>>>>> in the kprobe context (which is a debug trap). I'm not sure the arm64
>>>>> can cause NMI in that context, but if it happens and causes a fault,
>>>>> it may cause a problem.
>>>>>
>>>>> So I think we can just ignore the fault on the simulated kprobes.
>>>>>
>>>>> To ensure that, you can just add:
>>>>>
>>>>>            if (cur && !cur->ainsn.xol_insn)
>>>>>                    return 0;
>>>>>
>>>>> at the entry of this function. (and remove redundant cur->ainsn.xol_insn check)
>>>>
>>>> Right, both cases:
>>>>
>>>> 1. single-step XOL
>>>> 2. simulated
>>>>
>>>> have this problem and this patch fixed 1. 2 remains unchanged.
>>>>
>>>> Ideally we should fix both, but the simulated case seems more
>>>> complicated, and at least we didn't make things worse for 2. So I wonder
>>>> if we can analyze 2 more thoroughly and fix it in a separate patch.
>>>
>>> OK, but basically this fault handler is only for the SS XOL,
>>> not for simulated one (as same as x86). So just skip the
>>> simulated case is enough in this patch.
>>> (Note that x86 also have simulated path, and that is not handled
>>> by the fault handler)
>>
>> Hmm, what happens if the original PC is a simulated instruction that has
>> a recoverable ex_table entry that handles potential page fault,
>
> That should never happen. BPF trampoline code may populate extable
> entries, but kprobe doesn't. For the simulated instruction, as far
> as I can see, there are 2 operations
>   - simulate_ldr_literal
>   - simulate_ldrsw_literal
> will involve the memory access and both are accessing PC-relative
> kernel data, which should be mapped.
>
> load_addr = addr + ldr_displacement(opcode);
> ...
> set_x_reg(regs, xn, READ_ONCE(*(u64 *)load_addr));
>
> this directly accessing the memory without using extable.
>
>
>> and this
>> PC also has an attached kprobe?
>
> You meant that putting kprobes in simulate_* functions?
> Those have __kprobes attribute, so kprobe can not probe it.
> (It should use NOKPROBE_SYMBOL...)
>
>> Then the simulated case should be able
>> to enter kprobe_fault_handler()?
>>
>> .ex_table:
>>          insn foo, handler bar
>>
>> foo:
>>          LDR symbol # Load PC-relative. Simulated. Kprobe attached.
>>          ret
>>
>> When foo is called and the kprobe fires, it enters simulated path but
>> then LDR triggers a page fault. It then enters kprobe_fault_handler() to
>> fixup the PC. Then the fixup_exception() of the normal page fault finds
>> the ex_table entry and this case is successfully handled?
>
> To do that, you need to update the simulate_ldr* to use uaccess API
> (_ASM_EXTABLE_UACCESS* macros) AND allow kprobes to probe those functions.

I see. The kprobe code actually does not even allow this combination.
The arm64 arch_prepare_kprobe() will reject extable PCs. Sorry for the
noise.

>>
>> Looks like this handler can be entered by simulated instructions? Or
>> this is only theoretical and whoever arrange code like this should be
>> fired immediately?
>
> So the simulated instructions never cause fault, or if it causes a fault
> that means the original code has a bug. (Of course there is room to
> improve the bug message so that it decodes the address probed by kprobe
> when a bug occurs.)

If we rule out the edge case I came up with (which arm64 kprobe does not
even allow to happen), then other cases are indeed kernel bugs.

Thanks for walking us through some of the logic. Will fix things in v2.

  reply	other threads:[~2026-07-08 12:13 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 12:14 [RFC 0/2] arm64: kprobes: Fix single-step fault and reentry handling Pu Hu
2026-07-01 12:30 ` Pu Hu
2026-07-01 13:43 ` Masami Hiramatsu
2026-07-01 13:56   ` Pu Hu
2026-07-02 10:07     ` Pu Hu
2026-07-02 10:09       ` Pu Hu
2026-07-04 14:47         ` Masami Hiramatsu
     [not found]           ` <20260706083636.159883-1-hupu@transsion.com>
     [not found]             ` <20260706083636.159883-2-hupu@transsion.com>
2026-07-08  0:46               ` [RFC 1/3] arm64: kprobes: Do not handle non-XOL faults as kprobe faults Masami Hiramatsu
2026-07-08  5:57                 ` Hongyan Xia
2026-07-08  7:42                   ` Masami Hiramatsu
2026-07-08  8:55                     ` Hongyan Xia
2026-07-08 11:59                       ` Masami Hiramatsu
2026-07-08 12:12                         ` Hongyan Xia [this message]
     [not found]             ` <20260706083636.159883-3-hupu@transsion.com>
2026-07-08  0:54               ` [RFC 2/3] arm64: kprobes: Allow reentering kprobes while single-stepping Masami Hiramatsu
2026-07-08  7:12                 ` Hongyan Xia
2026-07-08 14:12                   ` Masami Hiramatsu
2026-07-08  0:55             ` [RFC 0/3] arm64: kprobes: Fix single-step fault and reentry handling Masami Hiramatsu

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=89a5b9a3-0bdf-4781-bfc3-9f6fe88e070d@transsion.com \
    --to=hongyan.xia@transsion.com \
    --cc=catalin.marinas@arm.com \
    --cc=davem@davemloft.net \
    --cc=hupu@transsion.com \
    --cc=jiazi.li@transsion.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=naveen@kernel.org \
    --cc=will@kernel.org \
    --cc=yang@os.amperecomputing.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox