Thread (3 messages) 3 messages, 2 authors, 2021-01-20

Re: [PATCH] powerpc/uprobes: Don't allow probe on suffix of prefixed instruction

From: Oleg Nesterov <oleg@redhat.com>
Date: 2021-01-19 18:29:22
Also in: lkml

On 01/19, Ravi Bangoria wrote:
Probe on 2nd word of a prefixed instruction is invalid scenario and
should be restricted.
I don't understand this ppc-specific problem, but...
+#ifdef CONFIG_PPC64
+int arch_uprobe_verify_opcode(struct page *page, unsigned long vaddr,
+			      uprobe_opcode_t opcode)
+{
+	uprobe_opcode_t prefix;
+	void *kaddr;
+	struct ppc_inst inst;
+
+	/* Don't check if vaddr is pointing to the beginning of page */
+	if (!(vaddr & ~PAGE_MASK))
+		return 0;
So the fix is incomplete? Or insn at the start of page can't be prefixed?
quoted hunk ↗ jump to hunk
+int __weak arch_uprobe_verify_opcode(struct page *page, unsigned long vaddr,
+				     uprobe_opcode_t opcode)
+{
+	return 0;
+}
+
 static int verify_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t *new_opcode)
 {
 	uprobe_opcode_t old_opcode;
@@ -275,6 +281,8 @@ static int verify_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t
 	if (is_swbp_insn(new_opcode)) {
 		if (is_swbp)		/* register: already installed? */
 			return 0;
+		if (arch_uprobe_verify_opcode(page, vaddr, old_opcode))
+			return -EINVAL;
Well, this doesn't look good...

To me it would be better to change the prepare_uprobe() path to copy
the potential prefix into uprobe->arch and check ppc_inst_prefixed()
in arch_uprobe_analyze_insn(). What do you think?

Oleg.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help