From: Eric Biggers <ebiggers@kernel.org>
To: David Herrmann <dh.herrmann@googlemail.com>,
Jiri Kosina <jikos@kernel.org>,
Benjamin Tissoires <benjamin.tissoires@redhat.com>,
linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
Dmitry Vyukov <dvyukov@google.com>,
Dmitry Torokhov <dtor@google.com>,
syzbot+72473edc9bf4eb1c6556@syzkaller.appspotmail.com,
stable@vger.kernel.org, Jann Horn <jannh@google.com>,
Andy Lutomirski <luto@kernel.org>
Subject: [PATCH v2] HID: uhid: forbid UHID_CREATE under KERNEL_DS or elevated privileges
Date: Wed, 14 Nov 2018 13:55:09 -0800 [thread overview]
Message-ID: <20181114215509.163600-1-ebiggers@kernel.org> (raw)
In-Reply-To: <CAG48ez3bPkh+DMPwiebM+r4ozX2CiVY=9=WMBP_xm1qVaSN4sQ@mail.gmail.com>
From: Eric Biggers <ebiggers@google.com>
When a UHID_CREATE command is written to the uhid char device, a
copy_from_user() is done from a user pointer embedded in the command.
When the address limit is KERNEL_DS, e.g. as is the case during
sys_sendfile(), this can read from kernel memory. Alternatively,
information can be leaked from a setuid binary that is tricked to write
to the file descriptor. Therefore, forbid UHID_CREATE in these cases.
No other commands in uhid_char_write() are affected by this bug and
UHID_CREATE is marked as "obsolete", so apply the restriction to
UHID_CREATE only rather than to uhid_char_write() entirely.
Thanks to Dmitry Vyukov for adding uhid definitions to syzkaller and to
Jann Horn for commit 9da3f2b740544 ("x86/fault: BUG() when uaccess
helpers fault on kernel addresses"), allowing this bug to be found.
Reported-by: syzbot+72473edc9bf4eb1c6556@syzkaller.appspotmail.com
Fixes: d365c6cfd337 ("HID: uhid: add UHID_CREATE and UHID_DESTROY events")
Cc: <stable@vger.kernel.org> # v3.6+
Cc: Jann Horn <jannh@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/hid/uhid.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 3c55073136064..051639c09f728 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -12,6 +12,7 @@
#include <linux/atomic.h>
#include <linux/compat.h>
+#include <linux/cred.h>
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/hid.h>
@@ -722,6 +723,17 @@ static ssize_t uhid_char_write(struct file *file, const char __user *buffer,
switch (uhid->input_buf.type) {
case UHID_CREATE:
+ /*
+ * 'struct uhid_create_req' contains a __user pointer which is
+ * copied from, so it's unsafe to allow this with elevated
+ * privileges (e.g. from a setuid binary) or via kernel_write().
+ */
+ if (file->f_cred != current_cred() || uaccess_kernel()) {
+ pr_err_once("UHID_CREATE from different security context by process %d (%s), this is not allowed.\n",
+ task_tgid_vnr(current), current->comm);
+ ret = -EACCES;
+ goto unlock;
+ }
ret = uhid_dev_create(uhid, &uhid->input_buf);
break;
case UHID_CREATE2:
--
2.19.1.930.g4563a0d9d0-goog
next prev parent reply other threads:[~2018-11-14 21:55 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-11 18:26 BUG: GPF in non-whitelisted uaccess (non-canonical address?) syzbot
2018-11-14 0:25 ` syzbot
2018-11-14 12:20 ` David Herrmann
2018-11-14 16:52 ` Dmitry Vyukov
2018-11-14 17:14 ` Eric Biggers
2018-11-14 18:02 ` [PATCH] HID: uhid: prevent uhid_char_write() under KERNEL_DS Eric Biggers
2018-11-14 18:14 ` Dmitry Torokhov
2018-11-14 18:18 ` Jann Horn
2018-11-14 21:54 ` Eric Biggers
2018-11-14 21:55 ` Eric Biggers [this message]
2018-11-14 22:04 ` [PATCH v2] HID: uhid: forbid UHID_CREATE under KERNEL_DS or elevated privileges Jann Horn
2018-11-14 22:28 ` Dmitry Torokhov
2018-11-14 22:37 ` Jann Horn
2018-11-14 22:46 ` Dmitry Torokhov
2018-11-15 0:39 ` Andy Lutomirski
2018-11-14 23:00 ` Eric Biggers
2018-11-14 23:20 ` Dmitry Torokhov
2018-11-15 8:14 ` Benjamin Tissoires
2018-11-15 12:06 ` David Herrmann
2018-11-15 14:50 ` Andy Lutomirski
2018-11-15 12:09 ` David Herrmann
2018-11-15 14:49 ` Andy Lutomirski
2018-11-19 12:52 ` Jiri Kosina
2018-11-19 13:21 ` David Herrmann
2018-11-19 13:26 ` Jiri Kosina
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=20181114215509.163600-1-ebiggers@kernel.org \
--to=ebiggers@kernel.org \
--cc=benjamin.tissoires@redhat.com \
--cc=dh.herrmann@googlemail.com \
--cc=dtor@google.com \
--cc=dvyukov@google.com \
--cc=jannh@google.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=stable@vger.kernel.org \
--cc=syzbot+72473edc9bf4eb1c6556@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.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.