Hi Nam,

On 19/11/2024 12:10, Nam Cao wrote:

p->ainsn.api.insn is a pointer to u32, therefore arithmetic operations are multiplied by four. This is clearly undesirable for this case.

Cast it to (void *) first before any calculation.

Below is a sample before/after. The dumped memory is two kprobe slots, the first slot has

• c.addiw a0, 0x1c (0x7125)
• ebreak (0x00100073)

and the second slot has:

• c.addiw a0, -4 (0x7135)
• ebreak (0x00100073)

Before this patch:

(gdb) x/16xh 0xff20000000135000 0xff20000000135000: 0x7125 0x0000 0x0000 0x0000 0x7135 0x0010 0x0000 0x0000 0xff20000000135010: 0x0073 0x0010 0x0000 0x0000 0x0000 0x0000 0x0000 0x0000

After this patch:

(gdb) x/16xh 0xff20000000125000 0xff20000000125000: 0x7125 0x0073 0x0010 0x0000 0x7135 0x0073 0x0010 0x0000 0xff20000000125010: 0x0000 0x0000 0x0000 0x0000 0x0000 0x0000 0x0000 0x0000

Fixes: b1756750a397 ("riscv: kprobes: Use patch_text_nosync() for insn slots") Signed-off-by: Nam Cao namcao@linutronix.de Cc: stable@vger.kernel.org

---

arch/riscv/kernel/probes/kprobes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 474a65213657..d2dacea1aedd 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -30,7 +30,7 @@ static void __kprobes arch_prepare_ss_slot(struct kprobe *p) p->ainsn.api.restore = (unsigned long)p->addr + len; patch_text_nosync(p->ainsn.api.insn, &p->opcode, len);

• patch_text_nosync(p->ainsn.api.insn + len, &insn, GET_INSN_LENGTH(insn));

• patch_text_nosync((void *)p->ainsn.api.insn + len, &insn, GET_INSN_LENGTH(insn)); }

static void __kprobes arch_prepare_simulate(struct kprobe *p)

This looks good to me, how did you find this issue?

You can add:

Reviewed-by: Alexandre Ghiti alexghiti@rivosinc.com

Thanks,

Alex