[PATCH] riscv: kprobes: fix placement in arch_prepare_ss_slot

[Alexandre Ghiti]

Hi Ben,

On 16/12/2024 14:05, Ben Dooks wrote:
> The second patch_text_nosync() in arch_prepare_ss_slot should be pointing
> after the first instruction, however p->ainsn.api.insn is a pointer to a
> probe_opcode_t which is 4 bytes... this means an instruction of length 2
> moves the pointer by 8 bytes.
>
> This means a kprobe may fail to work and recurse into the bad instruction
> handler (which then itself fails as the original lock still seems to be
> held?)
>
> Fixes: b1756750a397f36ddc857 ("riscv: kprobes: Use patch_text_nosync() for insn slots")
> Signed-off-by: Ben Dooks <ben.dooks at codethink.co.uk>
> Cc: Samuel Holland <samuel.holland at sifive.com>
> ---
>   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 380a0e8cecc0..c0738d6c6498 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)


Nam already came up with the same fix a few weeks ago: 
https://lore.kernel.org/linux-riscv/20241119111056.2554419-1-namcao@linutronix.de/

Thanks anyway!

Alex