From: Meng Xu <meng.xu@gatech.edu>
To: peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
alexander.shishkin@linux.intel.com, linux-kernel@vger.kernel.org
Cc: meng.xu@gatech.edu, sanidhya@gatech.edu, taesoo@gatech.edu,
Meng Xu <mengxu.gatech@gmail.com>
Subject: [PATCH] perf/core: fix potential double-fetch bug
Date: Wed, 23 Aug 2017 17:07:50 -0400 [thread overview]
Message-ID: <1503522470-35531-1-git-send-email-meng.xu@gatech.edu> (raw)
From: Meng Xu <mengxu.gatech@gmail.com>
While examining the kernel source code, I found a dangerous operation that
could turn into a double-fetch situation (a race condition bug) where the same
userspace memory region are fetched twice into kernel with sanity checks after
the first fetch while missing checks after the second fetch.
1. The first fetch happens in line 9573 get_user(size, &uattr->size).
2. Subsequently the `size` variable undergoes a few sanity checks and
transformations (line 9577 to 9584).
3. The second fetch happens in line 9610 copy_from_user(attr, uattr, size)
4. Given that `uattr` can be fully controlled in userspace, an attacker can
race condition to override `uattr->size` to arbitrary value (say, 0xFFFFFFFF)
after the first fetch but before the second fetch. The changed value will be
copied to `attr->size`.
5. There is no further checks on `attr->size` until the end of this function,
and once the function returns, we lose the context to verify that `attr->size`
conforms to the sanity checks performed in step 2 (line 9577 to 9584).
6. My manual analysis shows that `attr->size` is not used elsewhere later,
so, there is no working exploit against it right now. However, this could
easily turns to an exploitable one if careless developers start to use
`attr->size` later.
Proposed patch:
The patch is a one-liner which overrides `attr->size` from the second fetch to
the one from the first fetch, regardless of what is actually copied in.
In this way, it is assured that `attr->size` is in consistent with the checks
performed after the first fetch.
Signed-off-by: Meng Xu <mengxu.gatech@gmail.com>
---
kernel/events/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ee20d4c..c0d7946 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9611,6 +9611,8 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
if (ret)
return -EFAULT;
+ attr->size = size;
+
if (attr->__reserved_1)
return -EINVAL;
--
2.7.4
next reply other threads:[~2017-08-23 21:17 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-23 21:07 Meng Xu [this message]
2017-08-29 13:06 ` [tip:perf/urgent] perf/core: Fix " tip-bot for Meng Xu
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=1503522470-35531-1-git-send-email-meng.xu@gatech.edu \
--to=meng.xu@gatech.edu \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mengxu.gatech@gmail.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=sanidhya@gatech.edu \
--cc=taesoo@gatech.edu \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®