All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
To: mpe@ellerman.id.au
Cc: ravi.bangoria@linux.ibm.com, oleg@redhat.com,
	rostedt@goodmis.org, paulus@samba.org, jniethe5@gmail.com,
	naveen.n.rao@linux.ibm.com, sandipan@linux.ibm.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: [PATCH] powerpc/uprobes: Don't allow probe on suffix of prefixed instruction
Date: Tue, 19 Jan 2021 14:42:34 +0530	[thread overview]
Message-ID: <20210119091234.76317-1-ravi.bangoria@linux.ibm.com> (raw)

Probe on 2nd word of a prefixed instruction is invalid scenario and
should be restricted.

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 we notice
that probe is on the 2nd word of 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 <ravi.bangoria@linux.ibm.com>
---
 arch/powerpc/kernel/uprobes.c | 28 ++++++++++++++++++++++++++++
 include/linux/uprobes.h       |  1 +
 kernel/events/uprobes.c       |  8 ++++++++
 3 files changed, 37 insertions(+)

diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index e8a63713e655..c73d5a397164 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -7,6 +7,7 @@
  * Adapted from the x86 port by Ananth N Mavinakayanahalli <ananth@in.ibm.com>
  */
 #include <linux/kernel.h>
+#include <linux/highmem.h>
 #include <linux/sched.h>
 #include <linux/ptrace.h>
 #include <linux/uprobes.h>
@@ -44,6 +45,33 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
 	return 0;
 }
 
+#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;
+
+	kaddr = kmap_atomic(page);
+	memcpy(&prefix, kaddr + ((vaddr - 4) & ~PAGE_MASK), UPROBE_SWBP_INSN_SIZE);
+	kunmap_atomic(kaddr);
+
+	inst = ppc_inst_prefix(prefix, opcode);
+
+	if (ppc_inst_prefixed(inst)) {
+		printk_ratelimited("Cannot register a uprobe on the second "
+				   "word of prefixed instruction\n");
+		return -1;
+	}
+	return 0;
+}
+#endif
+
 /*
  * arch_uprobe_pre_xol - prepare to execute out of line.
  * @auprobe: the probepoint information.
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index f46e0ca0169c..5a3b45878e13 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -128,6 +128,7 @@ extern bool uprobe_deny_signal(void);
 extern bool arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void uprobe_clear_state(struct mm_struct *mm);
 extern int  arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long addr);
+int arch_uprobe_verify_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t opcode);
 extern int  arch_uprobe_pre_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index bf9edd8d75be..be02e6c26e3f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -255,6 +255,12 @@ static void copy_to_page(struct page *page, unsigned long vaddr, const void *src
 	kunmap_atomic(kaddr);
 }
 
+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;
 	} else {
 		if (!is_swbp)		/* unregister: was it changed by us? */
 			return 0;
-- 
2.26.2


WARNING: multiple messages have this Message-ID (diff)
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
To: mpe@ellerman.id.au
Cc: ravi.bangoria@linux.ibm.com, jniethe5@gmail.com, oleg@redhat.com,
	rostedt@goodmis.org, linux-kernel@vger.kernel.org,
	paulus@samba.org, sandipan@linux.ibm.com,
	naveen.n.rao@linux.ibm.com, linuxppc-dev@lists.ozlabs.org
Subject: [PATCH] powerpc/uprobes: Don't allow probe on suffix of prefixed instruction
Date: Tue, 19 Jan 2021 14:42:34 +0530	[thread overview]
Message-ID: <20210119091234.76317-1-ravi.bangoria@linux.ibm.com> (raw)

Probe on 2nd word of a prefixed instruction is invalid scenario and
should be restricted.

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 we notice
that probe is on the 2nd word of 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 <ravi.bangoria@linux.ibm.com>
---
 arch/powerpc/kernel/uprobes.c | 28 ++++++++++++++++++++++++++++
 include/linux/uprobes.h       |  1 +
 kernel/events/uprobes.c       |  8 ++++++++
 3 files changed, 37 insertions(+)

diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index e8a63713e655..c73d5a397164 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -7,6 +7,7 @@
  * Adapted from the x86 port by Ananth N Mavinakayanahalli <ananth@in.ibm.com>
  */
 #include <linux/kernel.h>
+#include <linux/highmem.h>
 #include <linux/sched.h>
 #include <linux/ptrace.h>
 #include <linux/uprobes.h>
@@ -44,6 +45,33 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
 	return 0;
 }
 
+#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;
+
+	kaddr = kmap_atomic(page);
+	memcpy(&prefix, kaddr + ((vaddr - 4) & ~PAGE_MASK), UPROBE_SWBP_INSN_SIZE);
+	kunmap_atomic(kaddr);
+
+	inst = ppc_inst_prefix(prefix, opcode);
+
+	if (ppc_inst_prefixed(inst)) {
+		printk_ratelimited("Cannot register a uprobe on the second "
+				   "word of prefixed instruction\n");
+		return -1;
+	}
+	return 0;
+}
+#endif
+
 /*
  * arch_uprobe_pre_xol - prepare to execute out of line.
  * @auprobe: the probepoint information.
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index f46e0ca0169c..5a3b45878e13 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -128,6 +128,7 @@ extern bool uprobe_deny_signal(void);
 extern bool arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void uprobe_clear_state(struct mm_struct *mm);
 extern int  arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long addr);
+int arch_uprobe_verify_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t opcode);
 extern int  arch_uprobe_pre_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index bf9edd8d75be..be02e6c26e3f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -255,6 +255,12 @@ static void copy_to_page(struct page *page, unsigned long vaddr, const void *src
 	kunmap_atomic(kaddr);
 }
 
+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;
 	} else {
 		if (!is_swbp)		/* unregister: was it changed by us? */
 			return 0;
-- 
2.26.2


             reply	other threads:[~2021-01-19  9:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-19  9:12 Ravi Bangoria [this message]
2021-01-19  9:12 ` [PATCH] powerpc/uprobes: Don't allow probe on suffix of prefixed instruction Ravi Bangoria
2021-01-19 17:26 ` Oleg Nesterov
2021-01-19 17:26   ` Oleg Nesterov
2021-01-20 11:18   ` Ravi Bangoria
2021-01-20 11:18     ` Ravi Bangoria

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210119091234.76317-1-ravi.bangoria@linux.ibm.com \
    --to=ravi.bangoria@linux.ibm.com \
    --cc=jniethe5@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=oleg@redhat.com \
    --cc=paulus@samba.org \
    --cc=rostedt@goodmis.org \
    --cc=sandipan@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.