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 v2] powerpc/uprobes: Validation for prefixed instruction Date: Thu, 4 Feb 2021 16:17:03 +0530 [thread overview] Message-ID: <20210204104703.273429-1-ravi.bangoria@linux.ibm.com> (raw) Don't allow Uprobe on 2nd word of a prefixed instruction. As per ISA 3.1, prefixed instruction should not cross 64-byte boundary. So don't allow Uprobe on such prefixed instruction as well. 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> --- v1: http://lore.kernel.org/r/20210119091234.76317-1-ravi.bangoria@linux.ibm.com v1->v2: - Instead of introducing new arch hook from verify_opcode(), use existing hook arch_uprobe_analyze_insn(). - Add explicit check for prefixed instruction crossing 64-byte boundary. If probe is on such instruction, throw an error. arch/powerpc/kernel/uprobes.c | 66 ++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c index e8a63713e655..485d19a2a31f 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> @@ -28,6 +29,69 @@ bool is_trap_insn(uprobe_opcode_t *insn) return (is_trap(*insn)); } +#ifdef CONFIG_PPC64 +static int get_instr(struct mm_struct *mm, unsigned long addr, u32 *instr) +{ + struct page *page; + struct vm_area_struct *vma; + void *kaddr; + unsigned int gup_flags = FOLL_FORCE | FOLL_SPLIT_PMD; + + if (get_user_pages_remote(mm, addr, 1, gup_flags, &page, &vma, NULL) <= 0) + return -EINVAL; + + kaddr = kmap_atomic(page); + *instr = *((u32 *)(kaddr + (addr & ~PAGE_MASK))); + kunmap_atomic(kaddr); + put_page(page); + return 0; +} + +static int validate_prefixed_instr(struct mm_struct *mm, unsigned long addr) +{ + struct ppc_inst inst; + u32 prefix, suffix; + + /* + * No need to check if addr is pointing to beginning of the + * page. Even if probe is on a suffix of page-unaligned + * prefixed instruction, hw will raise exception and kernel + * will send SIGBUS. + */ + if (!(addr & ~PAGE_MASK)) + return 0; + + if (get_instr(mm, addr, &prefix) < 0) + return -EINVAL; + if (get_instr(mm, addr + 4, &suffix) < 0) + return -EINVAL; + + inst = ppc_inst_prefix(prefix, suffix); + if (ppc_inst_prefixed(inst) && (addr & 0x3F) == 0x3C) { + printk_ratelimited("Cannot register a uprobe on 64 byte " + "unaligned prefixed instruction\n"); + return -EINVAL; + } + + suffix = prefix; + if (get_instr(mm, addr - 4, &prefix) < 0) + return -EINVAL; + + inst = ppc_inst_prefix(prefix, suffix); + if (ppc_inst_prefixed(inst)) { + printk_ratelimited("Cannot register a uprobe on the second " + "word of prefixed instruction\n"); + return -EINVAL; + } + return 0; +} +#else +static int validate_prefixed_instr(struct mm_struct *mm, unsigned long addr) +{ + return 0; +} +#endif + /** * arch_uprobe_analyze_insn * @mm: the probed address space. @@ -41,7 +105,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, if (addr & 0x03) return -EINVAL; - return 0; + return validate_prefixed_instr(mm, addr); } /* -- 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 v2] powerpc/uprobes: Validation for prefixed instruction Date: Thu, 4 Feb 2021 16:17:03 +0530 [thread overview] Message-ID: <20210204104703.273429-1-ravi.bangoria@linux.ibm.com> (raw) Don't allow Uprobe on 2nd word of a prefixed instruction. As per ISA 3.1, prefixed instruction should not cross 64-byte boundary. So don't allow Uprobe on such prefixed instruction as well. 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> --- v1: http://lore.kernel.org/r/20210119091234.76317-1-ravi.bangoria@linux.ibm.com v1->v2: - Instead of introducing new arch hook from verify_opcode(), use existing hook arch_uprobe_analyze_insn(). - Add explicit check for prefixed instruction crossing 64-byte boundary. If probe is on such instruction, throw an error. arch/powerpc/kernel/uprobes.c | 66 ++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c index e8a63713e655..485d19a2a31f 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> @@ -28,6 +29,69 @@ bool is_trap_insn(uprobe_opcode_t *insn) return (is_trap(*insn)); } +#ifdef CONFIG_PPC64 +static int get_instr(struct mm_struct *mm, unsigned long addr, u32 *instr) +{ + struct page *page; + struct vm_area_struct *vma; + void *kaddr; + unsigned int gup_flags = FOLL_FORCE | FOLL_SPLIT_PMD; + + if (get_user_pages_remote(mm, addr, 1, gup_flags, &page, &vma, NULL) <= 0) + return -EINVAL; + + kaddr = kmap_atomic(page); + *instr = *((u32 *)(kaddr + (addr & ~PAGE_MASK))); + kunmap_atomic(kaddr); + put_page(page); + return 0; +} + +static int validate_prefixed_instr(struct mm_struct *mm, unsigned long addr) +{ + struct ppc_inst inst; + u32 prefix, suffix; + + /* + * No need to check if addr is pointing to beginning of the + * page. Even if probe is on a suffix of page-unaligned + * prefixed instruction, hw will raise exception and kernel + * will send SIGBUS. + */ + if (!(addr & ~PAGE_MASK)) + return 0; + + if (get_instr(mm, addr, &prefix) < 0) + return -EINVAL; + if (get_instr(mm, addr + 4, &suffix) < 0) + return -EINVAL; + + inst = ppc_inst_prefix(prefix, suffix); + if (ppc_inst_prefixed(inst) && (addr & 0x3F) == 0x3C) { + printk_ratelimited("Cannot register a uprobe on 64 byte " + "unaligned prefixed instruction\n"); + return -EINVAL; + } + + suffix = prefix; + if (get_instr(mm, addr - 4, &prefix) < 0) + return -EINVAL; + + inst = ppc_inst_prefix(prefix, suffix); + if (ppc_inst_prefixed(inst)) { + printk_ratelimited("Cannot register a uprobe on the second " + "word of prefixed instruction\n"); + return -EINVAL; + } + return 0; +} +#else +static int validate_prefixed_instr(struct mm_struct *mm, unsigned long addr) +{ + return 0; +} +#endif + /** * arch_uprobe_analyze_insn * @mm: the probed address space. @@ -41,7 +105,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, if (addr & 0x03) return -EINVAL; - return 0; + return validate_prefixed_instr(mm, addr); } /* -- 2.26.2
next reply other threads:[~2021-02-04 10:49 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-04 10:47 Ravi Bangoria [this message] 2021-02-04 10:47 ` [PATCH v2] powerpc/uprobes: Validation for prefixed instruction Ravi Bangoria 2021-02-04 10:49 ` Ravi Bangoria 2021-02-04 10:49 ` Ravi Bangoria 2021-02-04 13:15 ` Naveen N. Rao 2021-02-04 13:15 ` Naveen N. Rao 2021-02-08 11:08 ` Ravi Bangoria 2021-02-08 11:08 ` Ravi Bangoria 2021-02-04 14:19 ` Ananth N Mavinakayanahalli 2021-02-04 13:08 ` Naveen N. Rao 2021-02-04 13:08 ` Naveen N. Rao 2021-02-04 16:12 ` Naveen N. Rao 2021-02-04 16:12 ` Naveen N. Rao 2021-02-08 11:10 ` Ravi Bangoria 2021-02-08 11:10 ` Ravi Bangoria 2021-02-08 11:13 ` Ravi Bangoria 2021-02-08 11:13 ` Ravi Bangoria 2021-02-06 18:06 ` Oleg Nesterov 2021-02-06 18:06 ` Oleg Nesterov 2021-02-08 11:06 ` Ravi Bangoria 2021-02-08 11:06 ` 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=20210204104703.273429-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.