Re: [RFC PATCH 6/9] arm64/kprobes: Make kprobe_fault_handler() noinstr

On 7/31/2026 11:57 PM, Mark Rutland wrote:
> On Mon, Jul 27, 2026 at 12:25:43PM +0000, Hongyan Xia wrote:
>> From: Hongyan Xia <[email protected]>
>>
>> kprobe_fault_handler() is reached from do_page_fault() when a fault
>> happens while a kprobe single-step is armed. Everything it calls (the
>> save/restore helpers, kprobes_restore_local_irqflag(),
>> reset_current_kprobe(), register access) is noinstr after the previous
>> patch, and being called from an instrumentable caller into a noinstr
>> callee is fine.
> 
> I think it's fine to mark this as noinstr.
> 
> I don't think we need kprobe_fault_handler() in the first place given we
> don't permit kprobes to be placed on instructions with an extable entry.
> Any instruction which can be probled cannot legitimately cause a fault.

I think so, too. If we cannot attach to ex_table entry instructions, 
then there is no point in handling a page fault. The only thing worth 
being in kprobe_fault_handler() is probably just a BUG(). Or, we could 
remove it entirely.

> That and kprobes_restore_local_irqflag() can't do the right thing w.r.t
> DAIF, since it only fiddles with the value in the pt_regs, and doesn't
> touch the live inherited DAIF value. If we need to fiddle with the live
> DAIF value, we must do that much earlier in the entry-common code.

This should be fine. It manipulates DAIF in the previous fault, not the 
live one, and the value will be picked up correctly on ERET. 
kprobes_{save/restore}_local_irqflag() doesn't change anything in the 
current exception, unlike local_irq_save().

Unless you mean something else?

> Mark.
> 
>> Signed-off-by: Hongyan Xia <[email protected]>
>> ---
>>   arch/arm64/kernel/probes/kprobes.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
>> index e9fa66fa4217..4172998d48d9 100644
>> --- a/arch/arm64/kernel/probes/kprobes.c
>> +++ b/arch/arm64/kernel/probes/kprobes.c
>> @@ -310,7 +310,7 @@ post_kprobe_handler(struct kprobe *cur, struct kprobe_ctlblk *kcb, struct pt_reg
>>        reset_current_kprobe();
>>   }
>>
>> -int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
>> +int noinstr kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
>>   {
>>        struct kprobe *cur = kprobe_running();
>>        struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>> --
>> 2.47.3
>>