I put a log statement in jail_open in the kernel and verified that it's not being hit the first time. This is symptomatic of the permissions on the jailed device node being wrong, i.e. root:root instead of root:chronos. Formerly I would have this issue because udev wasn't setting the right permissions when the device node appeared so maybe that's still happening. I would have expected that DeviceJailControl poking udev to give it the device path would get rid of this possibility, though. :(

The FUSE filesystem definitely does cause it to be created in time since it stats everything before descending into directories or calling any functions. If this wasn't true we'd be getting ENOENT instead of EACCES.

Interestingly enough, this doesn't seem to happen when I use device_jail_utility directly:

localhost ~ # device_jail_utility --add=/dev/bus/usb/001/002; sudo -u chronos od -tx1 /dev/jailed-189-1
[0601/184007:INFO:device_jail_utility.cc(90)] created jail at /dev/jailed-189-1
0000000 12 01 00 02 00 00 00 40 d1 18 e2 4e 28 02 01 02
0000020 03 01 09 02 3e 00 02 01 00 80 fa 09 04 00 00 03
0000040 ff ff 00 04 07 05 81 02 00 02 00 07 05 01 02 00
0000060 02 00 07 05 82 03 1c 00 06 09 04 01 00 02 ff 42
0000100 01 00 07 05 83 02 00 02 00 07 05 02 02 00 02 00
0000120

I would expect this to be equivalent to trying to open it from inside the container. Is there some propagation issue with the change of ownership not being noticed in the container's FS namespace until something pokes the device again?

i'm not sure what you mean by "propagation".  iiuc, the /dev in the container is a bind mount to the device jail fs /run/ dir.  if that's the case, any change you make in any namespace (like chown or chmod) is atomically visible in every other namespace.

Here's some log output from when I tried to directly cat a device twice:

[first cat]
2017-06-02T11:31:45.172572-07:00 INFO device_jail_fs[6126]: stat(/)
2017-06-02T11:31:45.172608-07:00 INFO device_jail_fs[6126]: jail_stat(/)
2017-06-02T11:31:45.172709-07:00 INFO device_jail_fs[6126]: stat(/bus)
2017-06-02T11:31:45.172732-07:00 INFO device_jail_fs[6126]: jail_stat(/bus)
2017-06-02T11:31:45.173543-07:00 INFO device_jail_fs[6126]: stat(/bus/usb)
2017-06-02T11:31:45.173779-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb)
2017-06-02T11:31:45.174188-07:00 INFO device_jail_fs[6126]: stat(/bus/usb/001)
2017-06-02T11:31:45.174216-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb/001)
2017-06-02T11:31:45.174355-07:00 INFO device_jail_fs[6126]: stat(/bus/usb/001/003)
2017-06-02T11:31:45.174376-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb/001/003)
2017-06-02T11:31:45.175110-07:00 INFO kernel: [  811.419440] add_jail_device: assigned new index 0 (devt 251:0)
2017-06-02T11:31:45.175133-07:00 INFO kernel: [  811.419460] add_jail_device: initializing device jailed-189-2
2017-06-02T11:31:45.175223-07:00 INFO device_jail_fs[6126]: jail /dev/jailed-189-2 has user: root group: root mode: 8576
2017-06-02T11:31:45.175366-07:00 INFO device_jail_fs[6126]: stat(/bus/usb/001/003)
2017-06-02T11:31:45.175389-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb/001/003)
2017-06-02T11:31:45.175717-07:00 INFO device_jail_fs[6126]: jail /dev/jailed-189-2 has user: root group: root mode: 8576
[second cat]
2017-06-02T11:31:50.217173-07:00 INFO device_jail_fs[6126]: stat(/)
2017-06-02T11:31:50.217214-07:00 INFO device_jail_fs[6126]: jail_stat(/)
2017-06-02T11:31:50.217457-07:00 INFO device_jail_fs[6126]: stat(/bus)
2017-06-02T11:31:50.217492-07:00 INFO device_jail_fs[6126]: jail_stat(/bus)
2017-06-02T11:31:50.217725-07:00 INFO device_jail_fs[6126]: stat(/bus/usb)
2017-06-02T11:31:50.217759-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb)
2017-06-02T11:31:50.218139-07:00 INFO device_jail_fs[6126]: stat(/bus/usb/001)
2017-06-02T11:31:50.218190-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb/001)
2017-06-02T11:31:50.218391-07:00 INFO device_jail_fs[6126]: stat(/bus/usb/001/003)
2017-06-02T11:31:50.218420-07:00 INFO device_jail_fs[6126]: jail_stat(/bus/usb/001/003)
2017-06-02T11:31:50.218813-07:00 INFO device_jail_fs[6126]: jail /dev/jailed-189-2 has user: root group: chronos mode: 8624
2017-06-02T11:31:50.293702-07:00 INFO permission_broker[1404]: ProcessPath(/dev/bus/usb/001/003)
2017-06-02T11:31:50.405608-07:00 INFO permission_broker[1404]:   AllowUsbDeviceRule: ALLOW
2017-06-02T11:31:50.405644-07:00 INFO permission_broker[1404]:   AllowTtyDeviceRule: IGNORE
2017-06-02T11:31:50.529762-07:00 INFO permission_broker[1404]:   DenyClaimedUsbDeviceRule: DENY
2017-06-02T11:31:50.529799-07:00 INFO permission_broker[1404]: Verdict for /dev/bus/usb/001/003: DENY

So, the device is root:root when the first use happens. :( It's correct by the second use, though. The modes are rendered as decimal nubmers because of laziness, but 8576 = 20600 and 8624 = 20660.

Looks like udev isn't holding up its end of the bargain.

Running "udevadm monitor" does show KERNEL and UDEV add events at the time of the first cat, but it might not be happening quickly enough.

what would the overhead look like if device jail just walked about possible nodes itself ?

Technically you could still race if we did that, if something else tried to request the device node right after it was created.

https://chromium-review.googlesource.com/c/522910/ will change this so it won't expose the node/return from stat calls until the udev rules have been run on it. There's some extra discussion on the CL about what it would take to solve this "right", which we should do eventually in case programs don't crawl /dev/bus/usb for devices, but which is a somewhat more invasive change and maybe not necessary for M60.

I think adb/fastboot don't run into this issue because they crawl the /dev/bus/usb directory multiple times, so it's good that we have vont to test this out.

i have a workaround in the extension for R60, so we don't need to worry about backporting any new work i don't think

Unless the workaround operates every time you plug a new device in, you can still be susceptible to this race, since it happens at DeviceJailControl::AddDevice time.

the way containers work today is that things are spun up & probed from scratch on every command (and then all torn down when the command finishes).  if we want to make adb server persistent, then that would be a concern, but also an easy one to workaround as you'd "just" have to query twice.

adb scans /dev/bus/usb multiple times as I said in comment #6 so we don't need to worry about it for the standalone adb container (otherwise it never would have worked to begin with, heh). Users of vont might notice, though.

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

commit a22070068dbbc47372ecefeaf9600bebc1d8f9d5
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat Jun 03 01:39:40 2017

container_utils: wait for device initialization

device_jail_fs will attempt to use a device immediately after
DeviceJailControl::AddDevice returns the path, but udev may
not have finished processing it by that point. As a result, it
might still have default permissions. Make sure the device is
fully initialized before returning it so device_jail_fs will get
accurate stat information upon use.

BUG= chromium:728857 
TEST=using vont:
  # od -tx1 /dev/bus/usb/XXX/YYY
  without first listing the usb directory, and check that
  permission_broker handles the jail request without having to
  retry

Change-Id: Id65a39d99f88da87336a7a3ee364738ffae332bb
Reviewed-on: https://chromium-review.googlesource.com/522910
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a22070068dbbc47372ecefeaf9600bebc1d8f9d5/container_utils/device_jail/device_jail_control.cc

Let me know if you want me to backport this to 60.

to clarify, adb does not scan /dev/bus/usb multiple times.  i added a hack to our container specifically to force a stat of all the nodes in /dev/bus/usb/ *before* we try to execute adb/fastboot itself.

if your CL:522910 makes things work under vont the first time (e.g. just running cat or od on a direct node), then that should make it work under adb/fastboot too.

we can keep this in M61 as it isn't blocked in M60 for early feedback/testing.