Re: [linux-6.6.y bugreport] riscv: kprobe crash as some patchs lost

[Nam Cao <namcao@linutronix.de>]

On Fri, Apr 25, 2025 at 08:09:21PM +0800, Kai Zhang wrote:
> Hi Nam,
> 
> I reported a riscv kprobe bug of linux-6.6.y. It seems that
> 03753bfacbc6(riscv: kprobes: Fix incorrect address calculation) should be
> reverted. There are a lot of changes of riscv kprobe in upstream. I'm not
> all in sure of my suggested fix. Will you kind to help?

Certainly.

> Thanks,
> laokz
> 
> On 4/25/2025 4:07 PM, Greg Kroah-Hartman wrote:
> > On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
> > > On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
> > > > On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
> > > > > In most recent linux-6.6.y tree,
> > > > > `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the
> > > > > obsolete code:
> > > > > 
> > > > >       u32 insn = __BUG_INSN_32;
> > > > >       unsigned long offset = GET_INSN_LENGTH(p->opcode);
> > > > >       p->ainsn.api.restore = (unsigned long)p->addr + offset;
> > > > >       patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1);
> > > > >       patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);
> > > > > 
> > > > > The last two 1s are wrong size of written instructions , which would lead to
> > > > > kernel crash, like `insmod kprobe_example.ko` gives:
> > > > > 
> > > > > [  509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130
> > > > > [  509.837606][    C5] handler_pre: <kernel_clone> p->addr =
> > > > > 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120
> > > > > [  509.839315][    C5] Oops - illegal instruction [#1]
> > > > > 
> > > > > 
> > > > > I've tried two patchs from torvalds tree and it didn't crash again:
> > > > > 
> > > > > 51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased)
> > > > > 13134cc94914 riscv: kprobes: Fix incorrect address calculation

Please don't revert this patch. It fixes another issue.

You are correct that the sizes of the instructions are wrong. It can still
happen to work if only one instruction is patched.

This bug is not specific to v6.6. It is in mainline as well. Therefore fix
patch should be sent to mainline, and then backport to v6.6.

Can you please verify if the below patch fixes your crash?

Best regards,
Nam

diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
index 4fbc70e823f0..dc431b965bc3 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -28,8 +28,8 @@ static void __kprobes arch_prepare_ss_slot(struct kprobe *p)
 
 	p->ainsn.api.restore = (unsigned long)p->addr + offset;
 
-	patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1);
-	patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);
+	patch_text_nosync(p->ainsn.api.insn, &p->opcode, offset);
+	patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, sizeof(insn));
 }
 
 static void __kprobes arch_prepare_simulate(struct kprobe *p)