Re: [PATCH 2/2] powerpc/uprobes: Reject uprobe on a system call instruction
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2022-01-25 11:46:31
Nicholas Piggin [off-list ref] writes:
Per the ISA, a Trace interrupt is not generated for a system call [vectored] instruction. Reject uprobes on such instructions as we are not emulating a system call [vectored] instruction anymore.
This should really be patch 1, otherwise there's a single commit window where we allow uprobes on sc but don't honour them.
quoted hunk ↗ jump to hunk
Signed-off-by: Naveen N. Rao <redacted> [np: Switch to pr_info_ratelimited] Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/ppc-opcode.h | 1 + arch/powerpc/kernel/uprobes.c | 6 ++++++ 2 files changed, 7 insertions(+)diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h index 9675303b724e..8bbe16ce5173 100644 --- a/arch/powerpc/include/asm/ppc-opcode.h +++ b/arch/powerpc/include/asm/ppc-opcode.h@@ -411,6 +411,7 @@ #define PPC_RAW_DCBFPS(a, b) (0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (4 << 21)) #define PPC_RAW_DCBSTPS(a, b) (0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (6 << 21)) #define PPC_RAW_SC() (0x44000002) +#define PPC_RAW_SCV() (0x44000001) #define PPC_RAW_SYNC() (0x7c0004ac) #define PPC_RAW_ISYNC() (0x4c00012c)diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c index c6975467d9ff..3779fde804bd 100644 --- a/arch/powerpc/kernel/uprobes.c +++ b/arch/powerpc/kernel/uprobes.c@@ -41,6 +41,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, if (addr & 0x03) return -EINVAL; + if (ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SC() || + ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SCV()) {
We should probably reject hypercall too? There's also a lot of reserved fields in `sc`, so doing an exact match like this risks missing instructions that are badly formed but the CPU will happily execute as `sc`. We'd obviously never expect to see those in compiler generated code, but it'd still be safer to mask. We could probably just reject opcode 17 entirely. And I guess for a subsequent patch, but we should be rejecting some others here as well shouldn't we? Like rfid etc. cheers
+ pr_info_ratelimited("Rejecting uprobe on system call instruction\n");
+ return -EINVAL;
+ }
+
if (cpu_has_feature(CPU_FTR_ARCH_31) &&
ppc_inst_prefixed(ppc_inst_read(auprobe->insn)) &&
(addr & 0x3f) == 60) {
--
2.23.0