Nicholas Piggin <npig...@gmail.com> 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.
> Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>
> [np: Switch to pr_info_ratelimited]
> Signed-off-by: Nicholas Piggin <npig...@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
