Thread (25 messages) 25 messages, 9 authors, 2018-11-19

Re: [PATCH v2] HID: uhid: forbid UHID_CREATE under KERNEL_DS or elevated privileges

From: Andy Lutomirski <luto@amacapital.net>
Date: 2018-11-15 14:50:10
Also in: lkml, stable

On Nov 15, 2018, at 4:06 AM, David Herrmann [off-list ref] wrote:

Hey

On Thu, Nov 15, 2018 at 9:14 AM Benjamin Tissoires
[off-list ref] wrote:
quoted
On Thu, Nov 15, 2018 at 12:20 AM Dmitry Torokhov [off-list ref] wrote:
quoted
quoted
I think it's best not to make
assumptions about how the interface will be used and to be consistent with how
other ->write() methods in the kernel handle the misfeature where a __user
pointer in the write() or read() payload is dereferenced.
I can see that you might want to check credentials, etc, if interface
can be accessed by unprivileged process, however is it a big no no for
uhid/userio/uinput devices.
Yep, any sane distribution would restrict the permissions of
uhid/userio/uinput to only be accessed by root. If that ever changes,
there is already an issue with the system and it was compromised
either by a terribly dizzy sysadmin.
UHID is safe to be used by a non-root user. This does not imply that
you should open up access to the world, but you are free to have a
dedicated group or user with access to uhid. I agree that in most
common desktop-scenarios you should not grant world-access to it,
though.
quoted
quoted
quoted
Temporarily switching
to USER_DS would only avoid one of the two problems.
So because of the above there is only one problem. If your system
opened access to uhid to random processes you have much bigger
problems than exposing some data from a suid binary. You can spam "rm
-rf .; rm -rf /" though uhid if there is interactive session
somewhere.
quoted
Do you think the proposed restrictions would actually break anything?
It would break if someone uses UHID_CREATE with sendpage. I do not
know if anyone does. If we were certain there are no users we'd simply
removed UHID_CREATE altogether.
AFAICT, there are 2 users of uhid:
- bluez for BLE devices (using HID over GATT)
- hid-replay for debugging.
There are several more (eg., android bt-broadcom), and UHID_CREATE is
actively used. Dropping support for it will break these use-cases.
Is the support story for these programs such that we could add a big scary warning and remove UHID_CREATE in a couple months?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help