I've checked Ubuntu 12.04 LTS (3.2 kernel), 14.04 LTS (3.13 kernel), 16.04 (4.4 kernel) and 17.10 (4.12 kernel). I checked series 16 snaps for rpi2 and bbb (both 4.4 kernels). In all cases, /sys/bus/pci/devices is a directory full of symlinks out to directories in /sys/devices. Because they are symlinks, AppArmor necessarily resolves them to absolute paths so we need at most a read on the directory, with the ** being meaningless (we just have /sys/devices/... rules instead), but harmless.
I looked in the nvidia abstraction for apparmor and this rule is not present. I checked apparmor policy for other projects and debs and there are no occurrences of '/sys/bus/usb/devices/...'. The interface already has rules out to/sys/devices/pci[0-9]*/**/... so the sysfs accesses should be covered.
This change would be fine on the systems I examined, and if there is a problem, it will be seen quickly and would be easy to revert. However, in investigating this further I came across https://www.kernel.org/doc/html/v4.11/admin-guide/sysfs-rules.html which states a few things:
"The kernel-exported sysfs exports internal kernel implementation details and depends on internal kernel structures and layout. It is agreed upon by the kernel developers that the Linux kernel does not provide a stable internal API. Therefore, there are aspects of the sysfs interface that may not be stable across kernel releases."
"There is only one valid place in sysfs where hierarchy can be examined and this is below: /sys/devices. It is planned that all device directories will end up in the tree below this directory."
"There are currently three places for classification of devices: /sys/block, /sys/class and /sys/bus. It is planned that these will not contain any device directories themselves, but only flat lists of symlinks pointing to the unified /sys/devices tree. All three places have completely different rules on how to access device information. It is planned to merge all three classification directories into one place at /sys/subsystem, following the layout of the bus directories. All buses and classes, including the converted block subsystem, will show up there. The devices belonging to a subsystem will create a symlink in the “devices” directory at /sys/subsystem//devices"
"If /sys/subsystem exists, /sys/bus, /sys/class and /sys/block can be ignored. If it does not exist, you always have to scan all three places, as the kernel is free to move a subsystem from one place to the other, as long as the devices are still reachable by the same subsystem name."
In all, I think there is a small amount of risk to implementing this change in a stable series, since there are many systems out there with many different kernels that may do different things that haven't been enumerated. Small because there is no historical policy for /sys/bus/pci/devices/... anywhere. The '/sys/bus/pci/devices/** r,' is clearly very broad and we should've tried examining it more closely and try to limit that to something more specific (or at the various least, documented it better).
I'm going to give this a +1 to see if in the unlikely event in candidate testing we see some denials and then see if we can get better rules out of it. If not, we can always simply change this to:
Historically, this rule was first introduced in 60ffe18#diff-6b578415ded16b60039c76b7f0ef8eacR35 (after first being added to the default template in 477d058#diff-a34e166c5b3016c122430c5884f41e9bR180) to support nvidia.
I've checked Ubuntu 12.04 LTS (3.2 kernel), 14.04 LTS (3.13 kernel), 16.04 (4.4 kernel) and 17.10 (4.12 kernel). I checked series 16 snaps for rpi2 and bbb (both 4.4 kernels). In all cases,
/sys/bus/pci/devicesis a directory full of symlinks out to directories in/sys/devices. Because they are symlinks, AppArmor necessarily resolves them to absolute paths so we need at most a read on the directory, with the**being meaningless (we just have/sys/devices/...rules instead), but harmless.I looked in the nvidia abstraction for apparmor and this rule is not present. I checked apparmor policy for other projects and debs and there are no occurrences of '
/sys/bus/usb/devices/...'. The interface already has rules out to/sys/devices/pci[0-9]*/**/...so the sysfs accesses should be covered.This change would be fine on the systems I examined, and if there is a problem, it will be seen quickly and would be easy to revert. However, in investigating this further I came across https://www.kernel.org/doc/html/v4.11/admin-guide/sysfs-rules.html which states a few things:
In all, I think there is a small amount of risk to implementing this change in a stable series, since there are many systems out there with many different kernels that may do different things that haven't been enumerated. Small because there is no historical policy for
/sys/bus/pci/devices/...anywhere. The '/sys/bus/pci/devices/** r,' is clearly very broad and we should've tried examining it more closely and try to limit that to something more specific (or at the various least, documented it better).I'm going to give this a +1 to see if in the unlikely event in candidate testing we see some denials and then see if we can get better rules out of it. If not, we can always simply change this to:
(the former should've always been there, the latter is reverting to what we had before).