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
next 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: linkBe 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.