Messages in this thread | | | From | Björn Töpel <> | Subject | Re: [PATCH] riscv: kprobe: Fixup kernel panic when probing an illegal position | Date | Wed, 01 Feb 2023 10:30:14 +0100 |
| |
Guo Ren <guoren@kernel.org> writes:
>> > + if (!arch_check_kprobe(p)) >> > + return -EILSEQ; >> > + >> > /* copy instruction */ >> > p->opcode = *p->addr; >> >> Not related to your patch, but this can also trigger a misaligned load. > After rereading the spec, misaligned load/store is not mandatory > supported. (Although my machines and qemu are correct)
Yes. It would be great if it could be turned on by QEMU (you can with Spike).
> So I need fixup: > > diff --git a/arch/riscv/kernel/probes/kprobes.c > b/arch/riscv/kernel/probes/kprobes.c > index 27f8960c321c..0c016d496746 100644 > --- a/arch/riscv/kernel/probes/kprobes.c > +++ b/arch/riscv/kernel/probes/kprobes.c > @@ -58,12 +58,14 @@ static bool __kprobes arch_check_kprobe(struct kprobe *p) > { > unsigned long tmp = (unsigned long)p->addr - p->offset; > unsigned long addr = (unsigned long)p->addr; > + kprobe_opcode_t opcode; > > while (tmp <= addr) { > if (tmp == addr) > return true; > > - tmp += GET_INSN_LENGTH(*(kprobe_opcode_t *)tmp); > + memcpy(&opcode, (void *)tmp, sizeof(kprobe_opcode_t)); > + tmp += GET_INSN_LENGTH(opcode);
I'd prefer sizeof(opcode).
> } > > return false; > @@ -80,7 +82,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p) > return -EILSEQ; > > /* copy instruction */ > - p->opcode = *p->addr; > + memcpy(&p->opcode, p->addr, sizeof(kprobe_opcode_t));
Same comment as above, and this would ideally be a separate patch.
Björn
| |