riscv_probe_decode_insn() reads the probed instruction with a raw
32-bit load:
probe_opcode_t insn = *addr;
If the instruction is compressed and sits in the last two bytes of a
module text page, this load crosses into the following vmalloc guard
page and faults. arch_prepare_kprobe() already copies the instruction
in two 16-bit steps into p->opcode, so pass that value by value instead
of reading the probed address again.
Do the same in arch_uprobe_analyze_insn() and arch_uprobe_skip_sstep(),
which still read auprobe->insn[] with a raw 32-bit load, for
consistency with arch_prepare_kprobe().
Link:
https://lore.kernel.org/all/[email protected]
Signed-off-by: Xiaofeng Yuan <[email protected]>
---
arch/riscv/kernel/probes/decode-insn.c | 4 +---
arch/riscv/kernel/probes/decode-insn.h | 2 +-
arch/riscv/kernel/probes/kprobes.c | 2 +-
arch/riscv/kernel/probes/uprobes.c | 11 ++++++++---
4 files changed, 11 insertions(+), 8 deletions(-)
Tested on QEMU riscv64: the in-tree kprobes KUnit tests pass, and a
userspace uprobe smoke test (compressed c.j and 32-bit jal probe sites)
exercises arch_uprobe_analyze_insn() and arch_uprobe_skip_sstep() with
the new two-step 16-bit reads. The cross-page fault cannot be
reproduced in QEMU (it needs a module text page of size % 4096 == 4094),
so it is argued statically and confirmed by disassembly that the raw
load is gone.
diff --git a/arch/riscv/kernel/probes/decode-insn.c
b/arch/riscv/kernel/probes/decode-insn.c
index 433d9035b..4a298de88 100644
--- a/arch/riscv/kernel/probes/decode-insn.c
+++ b/arch/riscv/kernel/probes/decode-insn.c
@@ -14,10 +14,8 @@
* INSN_GOOD_NO_SLOT If instruction is supported but doesn't use its slot.
*/
enum probe_insn __kprobes
-riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *api)
+riscv_probe_decode_insn(probe_opcode_t insn, struct arch_probe_insn *api)
{
- probe_opcode_t insn = *addr;
-
/*
* Reject instructions list:
*/
diff --git a/arch/riscv/kernel/probes/decode-insn.h
b/arch/riscv/kernel/probes/decode-insn.h
index 42269a7d6..0064bc76e 100644
--- a/arch/riscv/kernel/probes/decode-insn.h
+++ b/arch/riscv/kernel/probes/decode-insn.h
@@ -13,6 +13,6 @@ enum probe_insn {
};
enum probe_insn __kprobes
-riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *asi);
+riscv_probe_decode_insn(probe_opcode_t insn, struct arch_probe_insn *asi);
#endif /* _RISCV_KERNEL_KPROBES_DECODE_INSN_H */
diff --git a/arch/riscv/kernel/probes/kprobes.c
b/arch/riscv/kernel/probes/kprobes.c
index 9e2afabf9..d9274b530 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -85,7 +85,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
p->opcode |= (kprobe_opcode_t)(*insn) << 16;
/* decode instruction */
- switch (riscv_probe_decode_insn(p->addr, &p->ainsn.api)) {
+ switch (riscv_probe_decode_insn(p->opcode, &p->ainsn.api)) {
case INSN_REJECTED: /* insn not supported */
return -EINVAL;
diff --git a/arch/riscv/kernel/probes/uprobes.c
b/arch/riscv/kernel/probes/uprobes.c
index eb177d0ce..8c003b7de 100644
--- a/arch/riscv/kernel/probes/uprobes.c
+++ b/arch/riscv/kernel/probes/uprobes.c
@@ -32,12 +32,15 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
struct mm_struct *mm,
unsigned long addr)
{
probe_opcode_t opcode;
+ u16 *insn = (u16 *)&auprobe->insn[0];
- opcode = *(probe_opcode_t *)(&auprobe->insn[0]);
+ opcode = (probe_opcode_t)(*insn++);
+ if (GET_INSN_LENGTH(opcode) == 4)
+ opcode |= (probe_opcode_t)(*insn) << 16;
auprobe->insn_size = GET_INSN_LENGTH(opcode);
- switch (riscv_probe_decode_insn(&opcode, &auprobe->api)) {
+ switch (riscv_probe_decode_insn(opcode, &auprobe->api)) {
case INSN_REJECTED:
return -EINVAL;
@@ -96,7 +99,9 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe,
struct pt_regs *regs)
if (!auprobe->simulate)
return false;
- insn = *(probe_opcode_t *)(&auprobe->insn[0]);
+ insn = (probe_opcode_t)(*(u16 *)&auprobe->insn[0]);
+ if (auprobe->insn_size == 4)
+ insn |= (probe_opcode_t)(*(u16 *)&auprobe->insn[2]) << 16;
addr = instruction_pointer(regs);
if (auprobe->api.handler)
--
2.34.1
