New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 784936 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Use upstream implementation of USBDEVFS_DROP_PRIVILEGES

Project Member Reported by ejcaruso@chromium.org, Nov 14 2017

Issue description

An implementation of USBDEVFS_DROP_PRIVILEGES, which we use in permission_broker and device_jail, went upstream between kernels 4.4 and 4.12. The implementation looks slightly different as the ioctl now takes an argument (and this also results in a different ioctl number). We would like to move all of our kernels to using this implementation so that we don't have to try and handle two different interfaces in userspace. We will also need to expose a function for dropping privileges so device_jail can still use it without invoking the ioctl (since that is now guaranteed to fail without doing set_fs hackery).

This affects kernels 3.8, 3.10, 3.14, 3.18, and 4.4.
 
I might be going against the current here, but I don't see how backporting something to a kernel that's years old is easier than having a two-line if statement in userspace.

If the kernel backports are clean... maybe, but this sounds crazy to me.
I think they'll be clean since the upstream implementation is one patch of ~200 lines, and I would like to avoid epatching conditional compilation into linux-headers to deal with the ioctl number changing.
Fair enough -- godspeed on the 3.8 backport.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 17 2017

Labels: merge-merged-chromeos-4.12
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/df81d31d297898127c3c407cf81ecf3eb9b6ad35

commit df81d31d297898127c3c407cf81ecf3eb9b6ad35
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Fri Nov 17 05:07:42 2017

CHROMIUM: device_jail: use a drop_privileges function for lockdown

The chromium-specific version of USBDEVFS_DROP_PRIVILEGES didn't
take any arguments and just set the privileges_dropped bit on the
usb_dev_state, so we were able to just do the ioctl from jail_device
directly. However, the upstream variant also takes a bitset that is
copied in from userspace, so this will no longer work. Instead we
can expose a function which performs the equivalent operation to
the old ioctl and call that from jail_device.

BUG= chromium:784936 ,b:68964143
TEST=using device_jail_server; security_DeviceJail_Lockdown passes

Change-Id: I9a8a040fc77f0b2e130eefe3ebbfc26fad67ad17
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/769209
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/df81d31d297898127c3c407cf81ecf3eb9b6ad35/include/linux/usb.h
[modify] https://crrev.com/df81d31d297898127c3c407cf81ecf3eb9b6ad35/security/chromiumos/jail_device.c
[modify] https://crrev.com/df81d31d297898127c3c407cf81ecf3eb9b6ad35/drivers/usb/core/devio.c

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 28 2017

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/704e509e906b95aeb6417c85efffdf8858a87a38

commit 704e509e906b95aeb6417c85efffdf8858a87a38
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Nov 28 23:44:18 2017

CHROMIUM: device_jail: use a drop_privileges function for lockdown

The chromium-specific version of USBDEVFS_DROP_PRIVILEGES didn't
take any arguments and just set the privileges_dropped bit on the
usb_dev_state, so we were able to just do the ioctl from jail_device
directly. However, the upstream variant also takes a bitset that is
copied in from userspace, so this will no longer work. Instead we
can expose a function which performs the equivalent operation to
the old ioctl and call that from jail_device.

BUG= chromium:784936 ,b:68964143
TEST=using device_jail_server; security_DeviceJail_Lockdown passes

Change-Id: I9a8a040fc77f0b2e130eefe3ebbfc26fad67ad17
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/769209
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit df81d31d297898127c3c407cf81ecf3eb9b6ad35)
Reviewed-on: https://chromium-review.googlesource.com/780559

[modify] https://crrev.com/704e509e906b95aeb6417c85efffdf8858a87a38/include/linux/usb.h
[modify] https://crrev.com/704e509e906b95aeb6417c85efffdf8858a87a38/security/chromiumos/jail_device.c
[modify] https://crrev.com/704e509e906b95aeb6417c85efffdf8858a87a38/drivers/usb/core/devio.c

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 19 2017

Labels: merge-merged-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7a0423174ecb2c22f67306aff856813771f184c4

commit 7a0423174ecb2c22f67306aff856813771f184c4
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Dec 19 01:54:59 2017

Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit b51c935f461c15a277eeddd9d9aa5c69e11530da. We'll
be replacing this with an upstream version.

BUG= chromium:784936 
TEST=try with permission_broker

Change-Id: I2ebfa237ced773794fb52222159e75202eb548f2
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/832812

[modify] https://crrev.com/7a0423174ecb2c22f67306aff856813771f184c4/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/7a0423174ecb2c22f67306aff856813771f184c4/drivers/usb/core/devio.c

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 19 2017

Labels: merge-merged-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1e5a405956a9fb3b382f504727277b57d0d05632

commit 1e5a405956a9fb3b382f504727277b57d0d05632
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Dec 19 04:33:09 2017

Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit b6ffb15e5f52414bf1c038b6825e739ea16613d2. We'll
be replacing this with an upstream version.

BUG= chromium:784936 
TEST=try with permission_broker

Change-Id: I9eb9a6f2dd51007106ac9ba7901057d33faa0e2a
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/832810

[modify] https://crrev.com/1e5a405956a9fb3b382f504727277b57d0d05632/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/1e5a405956a9fb3b382f504727277b57d0d05632/drivers/usb/core/devio.c

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 19 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/68c06256797a49a90cd43eed3b28effdde38da9e

commit 68c06256797a49a90cd43eed3b28effdde38da9e
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Dec 19 04:33:04 2017

CHROMIUM: device_jail: use a drop_privileges function for lockdown

The chromium-specific version of USBDEVFS_DROP_PRIVILEGES didn't
take any arguments and just set the privileges_dropped bit on the
usb_dev_state, so we were able to just do the ioctl from jail_device
directly. However, the upstream variant also takes a bitset that is
copied in from userspace, so this will no longer work. Instead we
can expose a function which performs the equivalent operation to
the old ioctl and call that from jail_device.

BUG= chromium:784936 
TEST=using device_jail_server; security_DeviceJail_Lockdown passes

Change-Id: I8d8c901f0b3d429fc45962d45d951b881f99f0bc
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/833172

[modify] https://crrev.com/68c06256797a49a90cd43eed3b28effdde38da9e/include/linux/usb.h
[modify] https://crrev.com/68c06256797a49a90cd43eed3b28effdde38da9e/security/chromiumos/jail_device.c
[modify] https://crrev.com/68c06256797a49a90cd43eed3b28effdde38da9e/drivers/usb/core/devio.c

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c3b5c96ce89548e834cf587ab8d48b36c60dea0a

commit c3b5c96ce89548e834cf587ab8d48b36c60dea0a
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 03:49:27 2017

Reland "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit 7a0423174ecb2c22f67306aff856813771f184c4.

Reason for revert: All these patches need to go in together.

Original change's description:
> Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."
>
> This reverts commit b51c935f461c15a277eeddd9d9aa5c69e11530da. We'll
> be replacing this with an upstream version.
>
> BUG= chromium:784936 
> TEST=try with permission_broker
>
> Change-Id: I2ebfa237ced773794fb52222159e75202eb548f2
> Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
> Reviewed-on: https://chromium-review.googlesource.com/832812

Bug:  chromium:784936 
Change-Id: Iff61596b216b43f038a3606d7ef31598a3661877
Reviewed-on: https://chromium-review.googlesource.com/834809
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/c3b5c96ce89548e834cf587ab8d48b36c60dea0a/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/c3b5c96ce89548e834cf587ab8d48b36c60dea0a/drivers/usb/core/devio.c

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 20 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a66a767897f88382761890528f6f32d26608ddd0

commit a66a767897f88382761890528f6f32d26608ddd0
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 03:49:25 2017

CHROMIUM: device_jail: use a drop_privileges function for lockdown

The chromium-specific version of USBDEVFS_DROP_PRIVILEGES didn't
take any arguments and just set the privileges_dropped bit on the
usb_dev_state, so we were able to just do the ioctl from jail_device
directly. However, the upstream variant also takes a bitset that is
copied in from userspace, so this will no longer work. Instead we
can expose a function which performs the equivalent operation to
the old ioctl and call that from jail_device.

BUG= chromium:784936 
TEST=using device_jail_server; security_DeviceJail_Lockdown passes

Change-Id: Iedd27d546abf78fa1265f461916e9646fb35df51
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/a66a767897f88382761890528f6f32d26608ddd0/include/linux/usb.h
[modify] https://crrev.com/a66a767897f88382761890528f6f32d26608ddd0/security/chromiumos/jail_device.c
[modify] https://crrev.com/a66a767897f88382761890528f6f32d26608ddd0/drivers/usb/core/devio.c

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/391657a239f44190643fba9cb848dc721fe6d96a

commit 391657a239f44190643fba9cb848dc721fe6d96a
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 11:03:47 2017

Reland "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit 1e5a405956a9fb3b382f504727277b57d0d05632.

Reason for revert: All these patches need to go in together.

Original change's description:
> Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."
> 
> This reverts commit b6ffb15e5f52414bf1c038b6825e739ea16613d2. We'll
> be replacing this with an upstream version.
> 
> BUG= chromium:784936 
> TEST=try with permission_broker
> 
> Change-Id: I9eb9a6f2dd51007106ac9ba7901057d33faa0e2a
> Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
> Reviewed-on: https://chromium-review.googlesource.com/832810

Bug:  chromium:784936 
Change-Id: I814ca75891fc143afc4323e53fc27bbbf9664f14

[modify] https://crrev.com/391657a239f44190643fba9cb848dc721fe6d96a/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/391657a239f44190643fba9cb848dc721fe6d96a/drivers/usb/core/devio.c

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 20 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8bdbf8c0752c0c0ac12776ed1a04a1ac09f59a34

commit 8bdbf8c0752c0c0ac12776ed1a04a1ac09f59a34
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 20:53:52 2017

CHROMIUM: device_jail: use a drop_privileges function for lockdown

The chromium-specific version of USBDEVFS_DROP_PRIVILEGES didn't
take any arguments and just set the privileges_dropped bit on the
usb_dev_state, so we were able to just do the ioctl from jail_device
directly. However, the upstream variant also takes a bitset that is
copied in from userspace, so this will no longer work. Instead we
can expose a function which performs the equivalent operation to
the old ioctl and call that from jail_device.

BUG= chromium:784936 
TEST=using device_jail_server; security_DeviceJail_Lockdown passes

Change-Id: I21f9d8bd3eb23ed3f0ed1f0182402e36a9c982a9
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/8bdbf8c0752c0c0ac12776ed1a04a1ac09f59a34/include/linux/usb.h
[modify] https://crrev.com/8bdbf8c0752c0c0ac12776ed1a04a1ac09f59a34/security/chromiumos/jail_device.c
[modify] https://crrev.com/8bdbf8c0752c0c0ac12776ed1a04a1ac09f59a34/drivers/usb/core/devio.c

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d8c62831427d870fdaec2e823bdd9a6452b6993e

commit d8c62831427d870fdaec2e823bdd9a6452b6993e
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 21:31:56 2017

Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit 9efe152b4501bf652cc739b2b698fd289d8c736a. We'll
be replacing this with an upstream version.

BUG= chromium:784936 
TEST=try with permission_broker

Change-Id: I2c13860abfe5c5534c090edbffdb9e3e7627b1ff
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/832512

[modify] https://crrev.com/d8c62831427d870fdaec2e823bdd9a6452b6993e/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/d8c62831427d870fdaec2e823bdd9a6452b6993e/drivers/usb/core/devio.c

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2e62634ba8fa207e339ee1db88b15632510169bd

commit 2e62634ba8fa207e339ee1db88b15632510169bd
Author: Reilly Grant <reillyg@chromium.org>
Date: Wed Dec 20 21:32:00 2017

BACKPORT: usb: devio: Add ioctl to disallow detaching kernel USB drivers.

The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
relinquish the ability to issue other ioctls that may interfere with
other processes and drivers that have claimed an interface on the
device.

This commit also includes a simple utility to be able to test the
ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c

Example (with qemu-kvm's input device):

    $ lsusb
    ...
    Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd

    $ usb-devices
    ...
    C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=usbhid

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    OK: privileges dropped!
    Available options:
    [0] Exit now
    [1] Reset device. Should fail if device is in use
    [2] Claim 4 interfaces. Should succeed where not in use
    [3] Narrow interface permission mask
    Which option shall I run?: 1
    ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted)
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 0

After unbinding usbhid:

    $ usb-devices
    ...
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=(none)

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 2
    OK: claimed if 0
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 1
    OK: USBDEVFS_RESET succeeded
    Which test shall I run next?: 0

After unbinding usbhid and restricting the mask:

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 3
    Insert new mask: 0
    OK: privileges dropped!
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)

(Backported from upstream d883f52e1f6d...)
CQ-DEPEND=CL:832818
BUG= chromium:784936 

Change-Id: I3785657140244fbcab990a8ef72e9fcd808bd6db
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Reilly Grant <reillyg@chromium.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-on: https://chromium-review.googlesource.com/832513

[modify] https://crrev.com/2e62634ba8fa207e339ee1db88b15632510169bd/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/2e62634ba8fa207e339ee1db88b15632510169bd/drivers/usb/core/devio.c
[modify] https://crrev.com/2e62634ba8fa207e339ee1db88b15632510169bd/Documentation/DocBook/usb.tmpl
[add] https://crrev.com/2e62634ba8fa207e339ee1db88b15632510169bd/Documentation/usb/usbdevfs-drop-permissions.c

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/09195f65e7b994cefad4ebc463dbc3282d47c38a

commit 09195f65e7b994cefad4ebc463dbc3282d47c38a
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 21:33:10 2017

Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit b51c935f461c15a277eeddd9d9aa5c69e11530da. We'll
be replacing this with an upstream version.

BUG= chromium:784936 
TEST=try with permission_broker

Change-Id: I6141a3a6efab7ab8f677c1c23f30a0ba52de00f1
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/832812
Reviewed-on: https://chromium-review.googlesource.com/837507

[modify] https://crrev.com/09195f65e7b994cefad4ebc463dbc3282d47c38a/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/09195f65e7b994cefad4ebc463dbc3282d47c38a/drivers/usb/core/devio.c

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f4bafe4c4d7fa8e2b3b5cb4ca9c614c84249041b

commit f4bafe4c4d7fa8e2b3b5cb4ca9c614c84249041b
Author: Reilly Grant <reillyg@chromium.org>
Date: Wed Dec 20 21:33:13 2017

BACKPORT: usb: devio: Add ioctl to disallow detaching kernel USB drivers.

The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
relinquish the ability to issue other ioctls that may interfere with
other processes and drivers that have claimed an interface on the
device.

This commit also includes a simple utility to be able to test the
ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c

Example (with qemu-kvm's input device):

    $ lsusb
    ...
    Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd

    $ usb-devices
    ...
    C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=usbhid

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    OK: privileges dropped!
    Available options:
    [0] Exit now
    [1] Reset device. Should fail if device is in use
    [2] Claim 4 interfaces. Should succeed where not in use
    [3] Narrow interface permission mask
    Which option shall I run?: 1
    ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted)
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 0

After unbinding usbhid:

    $ usb-devices
    ...
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=(none)

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 2
    OK: claimed if 0
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 1
    OK: USBDEVFS_RESET succeeded
    Which test shall I run next?: 0

After unbinding usbhid and restricting the mask:

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 3
    Insert new mask: 0
    OK: privileges dropped!
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)

(Backported from upstream d883f52e1f6d...)
CQ-DEPEND=CL:832818
BUG= chromium:784936 

Change-Id: I33b7723c5b73b1189d9a7df5b6688d7ec9b8229a
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Reilly Grant <reillyg@chromium.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-on: https://chromium-review.googlesource.com/832813

[modify] https://crrev.com/f4bafe4c4d7fa8e2b3b5cb4ca9c614c84249041b/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/f4bafe4c4d7fa8e2b3b5cb4ca9c614c84249041b/drivers/usb/core/devio.c
[modify] https://crrev.com/f4bafe4c4d7fa8e2b3b5cb4ca9c614c84249041b/Documentation/DocBook/usb.tmpl
[add] https://crrev.com/f4bafe4c4d7fa8e2b3b5cb4ca9c614c84249041b/Documentation/usb/usbdevfs-drop-permissions.c

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/829f337126948b3482f19a95cef5ea30f606ef6a

commit 829f337126948b3482f19a95cef5ea30f606ef6a
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 21:33:33 2017

Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit b6ffb15e5f52414bf1c038b6825e739ea16613d2. We'll
be replacing this with an upstream version.

BUG= chromium:784936 
TEST=try with permission_broker

Change-Id: I2828946612b3d8a2cec586ae565ee9f4368841fb
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/832810
Reviewed-on: https://chromium-review.googlesource.com/837508

[modify] https://crrev.com/829f337126948b3482f19a95cef5ea30f606ef6a/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/829f337126948b3482f19a95cef5ea30f606ef6a/drivers/usb/core/devio.c

Project Member

Comment 18 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/feb48aabacb58fe04e585c967d8bd2c5cd65c937

commit feb48aabacb58fe04e585c967d8bd2c5cd65c937
Author: Reilly Grant <reillyg@chromium.org>
Date: Wed Dec 20 21:33:44 2017

BACKPORT: usb: devio: Add ioctl to disallow detaching kernel USB drivers.

The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
relinquish the ability to issue other ioctls that may interfere with
other processes and drivers that have claimed an interface on the
device.

This commit also includes a simple utility to be able to test the
ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c

Example (with qemu-kvm's input device):

    $ lsusb
    ...
    Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd

    $ usb-devices
    ...
    C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=usbhid

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    OK: privileges dropped!
    Available options:
    [0] Exit now
    [1] Reset device. Should fail if device is in use
    [2] Claim 4 interfaces. Should succeed where not in use
    [3] Narrow interface permission mask
    Which option shall I run?: 1
    ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted)
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 0

After unbinding usbhid:

    $ usb-devices
    ...
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=(none)

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 2
    OK: claimed if 0
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 1
    OK: USBDEVFS_RESET succeeded
    Which test shall I run next?: 0

After unbinding usbhid and restricting the mask:

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 3
    Insert new mask: 0
    OK: privileges dropped!
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)

(Backported from upstream d883f52e1f6d...)
CQ-DEPEND=CL:832818
BUG= chromium:784936 

Change-Id: I00aeae902e29118d9f9e7b8eb37e06f0ec0e2827
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Reilly Grant <reillyg@chromium.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-on: https://chromium-review.googlesource.com/832811

[modify] https://crrev.com/feb48aabacb58fe04e585c967d8bd2c5cd65c937/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/feb48aabacb58fe04e585c967d8bd2c5cd65c937/drivers/usb/core/devio.c
[modify] https://crrev.com/feb48aabacb58fe04e585c967d8bd2c5cd65c937/Documentation/DocBook/usb.tmpl
[add] https://crrev.com/feb48aabacb58fe04e585c967d8bd2c5cd65c937/Documentation/usb/usbdevfs-drop-permissions.c

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/9a149683495dcdef041eff839a699f63fccde3fa

commit 9a149683495dcdef041eff839a699f63fccde3fa
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 21:33:56 2017

Revert "CHROMIUM: Add ioctl to disallow detaching kernel USB drivers."

This reverts commit ccc078116664ea2728d282b0be6344842f6139ac. We'll
be replacing this with an upstream version.

BUG= chromium:784936 
TEST=try with permission_broker

Change-Id: Ic3755257e0e33b2733a382a7a5b8ca8b16a7b828
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/832816

[modify] https://crrev.com/9a149683495dcdef041eff839a699f63fccde3fa/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/9a149683495dcdef041eff839a699f63fccde3fa/drivers/usb/core/devio.c

Project Member

Comment 20 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/87dab778c712439473f7ef97c84e3df8ba38e0d1

commit 87dab778c712439473f7ef97c84e3df8ba38e0d1
Author: Reilly Grant <reillyg@chromium.org>
Date: Wed Dec 20 21:33:59 2017

UPSTREAM: usb: devio: Add ioctl to disallow detaching kernel USB drivers.

The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
relinquish the ability to issue other ioctls that may interfere with
other processes and drivers that have claimed an interface on the
device.

This commit also includes a simple utility to be able to test the
ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c

Example (with qemu-kvm's input device):

    $ lsusb
    ...
    Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd

    $ usb-devices
    ...
    C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=usbhid

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    OK: privileges dropped!
    Available options:
    [0] Exit now
    [1] Reset device. Should fail if device is in use
    [2] Claim 4 interfaces. Should succeed where not in use
    [3] Narrow interface permission mask
    Which option shall I run?: 1
    ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted)
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 0

After unbinding usbhid:

    $ usb-devices
    ...
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=(none)

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 2
    OK: claimed if 0
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 1
    OK: USBDEVFS_RESET succeeded
    Which test shall I run next?: 0

After unbinding usbhid and restricting the mask:

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 3
    Insert new mask: 0
    OK: privileges dropped!
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)

(Picked from upstream d883f52e1f6d...)
CQ-DEPEND=CL:832818
BUG= chromium:784936 

Change-Id: I5bf1f55c558e99a7458932544ec242d639d0b92c
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Reilly Grant <reillyg@chromium.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-on: https://chromium-review.googlesource.com/832817

[modify] https://crrev.com/87dab778c712439473f7ef97c84e3df8ba38e0d1/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/87dab778c712439473f7ef97c84e3df8ba38e0d1/drivers/usb/core/devio.c
[modify] https://crrev.com/87dab778c712439473f7ef97c84e3df8ba38e0d1/Documentation/DocBook/usb.tmpl
[add] https://crrev.com/87dab778c712439473f7ef97c84e3df8ba38e0d1/Documentation/usb/usbdevfs-drop-permissions.c

Project Member

Comment 21 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7d1b9e5f52b76df29e54e3bea4d691c5fdda916f

commit 7d1b9e5f52b76df29e54e3bea4d691c5fdda916f
Author: Reilly Grant <reillyg@chromium.org>
Date: Wed Dec 20 21:34:18 2017

UPSTREAM: usb: devio: Add ioctl to disallow detaching kernel USB drivers.

The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
relinquish the ability to issue other ioctls that may interfere with
other processes and drivers that have claimed an interface on the
device.

This commit also includes a simple utility to be able to test the
ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c

Example (with qemu-kvm's input device):

    $ lsusb
    ...
    Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd

    $ usb-devices
    ...
    C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=usbhid

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    OK: privileges dropped!
    Available options:
    [0] Exit now
    [1] Reset device. Should fail if device is in use
    [2] Claim 4 interfaces. Should succeed where not in use
    [3] Narrow interface permission mask
    Which option shall I run?: 1
    ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted)
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 0

After unbinding usbhid:

    $ usb-devices
    ...
    I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=(none)

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 2
    OK: claimed if 0
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)
    Which test shall I run next?: 1
    OK: USBDEVFS_RESET succeeded
    Which test shall I run next?: 0

After unbinding usbhid and restricting the mask:

    $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
    ...
    Which option shall I run?: 3
    Insert new mask: 0
    OK: privileges dropped!
    Which test shall I run next?: 2
    ERROR claiming if 0 (1 - Operation not permitted)
    ERROR claiming if 1 (1 - Operation not permitted)
    ERROR claiming if 2 (1 - Operation not permitted)
    ERROR claiming if 3 (1 - Operation not permitted)

(Picked from upstream d883f52e1f6d...)
CQ-DEPEND=CL:832818
BUG= chromium:784936 

Change-Id: Iedbe96733ce13fa033ba61c310723a19218f6ec4
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Reilly Grant <reillyg@chromium.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-on: https://chromium-review.googlesource.com/833174

[modify] https://crrev.com/7d1b9e5f52b76df29e54e3bea4d691c5fdda916f/include/uapi/linux/usbdevice_fs.h
[modify] https://crrev.com/7d1b9e5f52b76df29e54e3bea4d691c5fdda916f/drivers/usb/core/devio.c
[modify] https://crrev.com/7d1b9e5f52b76df29e54e3bea4d691c5fdda916f/Documentation/DocBook/usb.tmpl
[add] https://crrev.com/7d1b9e5f52b76df29e54e3bea4d691c5fdda916f/Documentation/usb/usbdevfs-drop-permissions.c

Project Member

Comment 22 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/61dc5242a8c8cb8784d94af4637eb301a4b80161

commit 61dc5242a8c8cb8784d94af4637eb301a4b80161
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Dec 20 21:34:25 2017

linux-headers: Use upstream USBDEVFS_DROP_PRIVILEGES

CQ-DEPEND=CL:832811,CL:832813,CL:832513,CL:832817,CL:833174
BUG= chromium:784936 
TEST=emerge

Change-Id: Ie0967b9dcd79abfd4d34c0c5553683bc1198fd19
Reviewed-on: https://chromium-review.googlesource.com/832818
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>

[add] https://crrev.com/61dc5242a8c8cb8784d94af4637eb301a4b80161/sys-kernel/linux-headers/files/0022-BACKPORT-Use-upstream-USBDEVFS_DROP_PRIVILEGES.patch
[modify] https://crrev.com/61dc5242a8c8cb8784d94af4637eb301a4b80161/sys-kernel/linux-headers/linux-headers-4.4.ebuild

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/24ee1503fbc0b56d6816f35cc5afc1bfa1b5646e

commit 24ee1503fbc0b56d6816f35cc5afc1bfa1b5646e
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Jan 03 05:32:17 2018

linux-headers, permission_broker: uprev and fix epatch

Looks like CL:832818 didn't uprev the linux-headers ebuild
revision number. USBDEVFS_DROP_PRIVILEGES never propagated
through to the system headers either because of a spurious
newline in the patch file. Also, since permission_broker is
going to need this macro it should depend on linux-headers
as well.

BUG= chromium:784936 
TEST=emerge

Change-Id: I34b2dd706511396d06020fb337cf646c249efcdf
Reviewed-on: https://chromium-review.googlesource.com/847682
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[rename] https://crrev.com/24ee1503fbc0b56d6816f35cc5afc1bfa1b5646e/sys-kernel/linux-headers/linux-headers-4.4-r12.ebuild
[modify] https://crrev.com/24ee1503fbc0b56d6816f35cc5afc1bfa1b5646e/chromeos-base/permission_broker/permission_broker-9999.ebuild
[modify] https://crrev.com/24ee1503fbc0b56d6816f35cc5afc1bfa1b5646e/sys-kernel/linux-headers/files/0022-BACKPORT-Use-upstream-USBDEVFS_DROP_PRIVILEGES.patch

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/80654acee3b3ebaacc8945caa39225acb66b7b20

commit 80654acee3b3ebaacc8945caa39225acb66b7b20
Author: Shirish S <shirish.s@amd.com>
Date: Thu Jan 04 00:54:35 2018

permission_broker: adapt ioctl to upstream variant

USBDEVFS_DROP_PRIVILEGES ioctl expects an argument
in kernel v4.14, without which it fails to set
privileges_dropped as true in the devio.c file of
kernel.

The change is taken after referring the below test file:
https://www.kernel.org/doc/Documentation/usb/usbdevfs-drop-permissions.c

CQ-DEPEND=CL:847682
BUG= chromium:784936 
TEST="test_that <DUT_IP> security_DeviceJail_Lockdown" passes

Change-Id: I27070c8d6ae073bf3536fd81eedf1047101e3f11
Signed-off-by: Shirish S <shirish.s@amd.com>
Reviewed-on: https://chromium-review.googlesource.com/765613
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/80654acee3b3ebaacc8945caa39225acb66b7b20/permission_broker/permission_broker.cc

Status: Fixed (was: Started)

Sign in to add a comment