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

[Nam Cao <namcao@linutronix.de>]

On Fri, Apr 25, 2025 at 02:49:52PM +0200, Nam Cao wrote:
> On Fri, Apr 25, 2025 at 08:09:21PM +0800, Kai Zhang wrote:
> > 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.

Sorry, I was confused. This is not in mainline. It has already been fixed
by 51781ce8f448 ("riscv: Pass patch_text() the length in bytes")

But I wouldn't backport that patch, it is bigger than necessary. The patch
I sent in the previous email should be enough.

Nam