From: Oleg Nesterov <oleg@redhat.com>
To: Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
Guo Ren <guoren@kernel.org>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"David S. Miller" <davem@davemloft.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Steven Rostedt <rostedt@goodmis.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Jann Horn <jannh@google.com>,
Al Viro <viro@zeniv.linux.org.uk>, Jens Axboe <axboe@kernel.dk>,
Security Officers <security@kernel.org>,
Andrea Arcangeli <aarcange@redhat.com>,
Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
Naveen Rao <naveen.n.rao@linux.vnet.ibm.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org
Subject: [PATCH] uprobes: ensure that uprobe->offset and ->ref_ctr_offset are properly aligned
Date: Mon, 4 May 2020 18:47:25 +0200 [thread overview]
Message-ID: <20200504164724.GA28697@redhat.com> (raw)
In-Reply-To: <20200428123914.GA27920@redhat.com>
uprobe_write_opcode() must not cross page boundary; prepare_uprobe()
relies on arch_uprobe_analyze_insn() which should validate "vaddr" but
some architectures (csky, s390, and sparc) don't do this.
We can remove the BUG_ON() check in prepare_uprobe() and validate the
offset early in __uprobe_register(). The new IS_ALIGNED() check matches
the alignment check in arch_prepare_kprobe() on supported architectures,
so I think that all insns must be aligned to UPROBE_SWBP_INSN_SIZE.
Another problem is __update_ref_ctr() which was wrong from the very
beginning, it can read/write outside of kmap'ed page unless "vaddr" is
aligned to sizeof(short), __uprobe_register() should check this too.
Cc: stable@vger.kernel.org
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/events/uprobes.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index ece7e13f6e4a..cc2095607c74 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -867,10 +867,6 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
if (ret)
goto out;
- /* uprobe_write_opcode() assumes we don't cross page boundary */
- BUG_ON((uprobe->offset & ~PAGE_MASK) +
- UPROBE_SWBP_INSN_SIZE > PAGE_SIZE);
-
smp_wmb(); /* pairs with the smp_rmb() in handle_swbp() */
set_bit(UPROBE_COPY_INSN, &uprobe->flags);
@@ -1166,6 +1162,15 @@ static int __uprobe_register(struct inode *inode, loff_t offset,
if (offset > i_size_read(inode))
return -EINVAL;
+ /*
+ * This ensures that copy_from_page(), copy_to_page() and
+ * __update_ref_ctr() can't cross page boundary.
+ */
+ if (!IS_ALIGNED(offset, UPROBE_SWBP_INSN_SIZE))
+ return -EINVAL;
+ if (!IS_ALIGNED(ref_ctr_offset, sizeof(short)))
+ return -EINVAL;
+
retry:
uprobe = alloc_uprobe(inode, offset, ref_ctr_offset);
if (!uprobe)
@@ -2014,6 +2019,9 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
uprobe_opcode_t opcode;
int result;
+ if (WARN_ON_ONCE(!IS_ALIGNED(vaddr, UPROBE_SWBP_INSN_SIZE)))
+ return -EINVAL;
+
pagefault_disable();
result = __get_user(opcode, (uprobe_opcode_t __user *)vaddr);
pagefault_enable();
--
2.25.1.362.g51ebf55
next parent reply other threads:[~2020-05-04 16:47 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAHk-=whQt69ApMkZF8b2Q2idMDgPpPETZeeOuZg59CrOO4025w@mail.gmail.com>
[not found] ` <20200428091149.GB19958@linux.vnet.ibm.com>
[not found] ` <20200428123914.GA27920@redhat.com>
2020-05-04 16:47 ` Oleg Nesterov [this message]
2020-05-04 18:40 ` [PATCH] uprobes: ensure that uprobe->offset and ->ref_ctr_offset are properly aligned Christian Borntraeger
2020-05-05 6:49 ` Sven Schnelle
2020-06-10 2:53 ` Guo Ren
2020-05-06 5:29 ` Srikar Dronamraju
2020-05-06 12:51 ` Steven Rostedt
2020-05-06 17:16 ` Oleg Nesterov
2020-06-09 15:30 ` Oleg Nesterov
2020-06-09 16:48 ` Linus Torvalds
2020-06-09 17:43 ` Steven Rostedt
2020-06-10 2:54 ` Guo Ren
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=20200504164724.GA28697@redhat.com \
--to=oleg@redhat.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ananth@in.ibm.com \
--cc=axboe@kernel.dk \
--cc=borntraeger@de.ibm.com \
--cc=davem@davemloft.net \
--cc=ebiederm@xmission.com \
--cc=guoren@kernel.org \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=naveen.n.rao@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=security@kernel.org \
--cc=srikar@linux.vnet.ibm.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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.