Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Naveen N. Rao <hidden>
Date: 2021-03-04 05:55:21
Also in:
lkml
On 2021/03/04 10:35AM, Ravi Bangoria wrote:
quoted hunk ↗ jump to hunk
As per ISA 3.1, prefixed instruction should not cross 64-byte boundary. So don't allow Uprobe on such prefixed instruction. There are two ways probed instruction is changed in mapped pages. First, when Uprobe is activated, it searches for all the relevant pages and replace instruction in them. In this case, if that probe is on the 64-byte unaligned prefixed instruction, error out directly. Second, when Uprobe is already active and user maps a relevant page via mmap(), instruction is replaced via mmap() code path. But because Uprobe is invalid, entire mmap() operation can not be stopped. In this case just print an error and continue. Signed-off-by: Ravi Bangoria <redacted> --- v2: https://lore.kernel.org/r/20210204104703.273429-1-ravi.bangoria@linux.ibm.com (local) v2->v3: - Drop restriction for Uprobe on suffix of prefixed instruction. It needs lot of code change including generic code but what we get in return is not worth it. arch/powerpc/kernel/uprobes.c | 8 ++++++++ 1 file changed, 8 insertions(+)diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c index e8a63713e655..c400971ebe70 100644 --- a/arch/powerpc/kernel/uprobes.c +++ b/arch/powerpc/kernel/uprobes.c@@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, if (addr & 0x03) return -EINVAL; + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31)) + return 0;
Sorry, I missed this last time, but I think we can drop the above checks. ppc_inst_prefixed() already factors in the dependency on CONFIG_PPC64, and I don't think we need to confirm if we're running on a ISA V3.1 for the below check. With that: Acked-by: Naveen N. Rao <redacted>
+
+ if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
+ pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
+ return -EINVAL;
+ }
+- Naveen