On Tue, Nov 22, 2022 at 9:55 AM Michal Prívozník <mprivozn@xxxxxxxxxx> wrote: > > On 11/22/22 09:47, Christian Ehrhardt wrote: > > On Mon, Nov 21, 2022 at 4:51 PM Michal Prívozník <mprivozn@xxxxxxxxxx> wrote: > >> > >> On 11/17/22 09:42, christian.ehrhardt@xxxxxxxxxxxxx wrote: > >>> From: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> > >>> > >>> For the handling of usb we already allow plenty of read access, > >>> but so far /sys/bus/usb/devices only needed read access to the directory > >>> to enumerate the symlinks in there that point to the actual entries via > >>> relative links to ../../../devices/. > >>> > >>> But in more recent systemd with updated libraries a program might do > >>> getattr calls on those symlinks. And while symlinks in apparmor usually > >>> do not matter, as it is the effective target of an access that has to be > >>> allowed, here the getattr calls are on the links themselves. > >>> > >>> On USB hostdev usage that causes a set of denials like: > >>> apparmor="DENIED" operation="getattr" class="file" > >>> name="/sys/bus/usb/devices/usb1" comm="qemu-system-x86" > >>> requested_mask="r" denied_mask="r" ... > >>> > >>> It is safe to read the links, therefore add a rule to allow it to > >>> the block of rules that covers the usb related access. > >>> > >>> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> > >>> --- > >>> src/security/apparmor/libvirt-qemu | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >> > >> Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > > > > Thank you for having a look, we are not yet in the 8.10 freeze and the > > case is rather straightforward, therefore I have pushed it now. > > Perfect. But just to clarify what freeze means: it's a period where we > try to stabilize for the release. E.g. I run various (hand) tests, try > more exotic operations that I don't do daily. Now, should we find a bug, > merging its fix is very much desired. But merging a feature is less so, > because that usually comes with introducing a bug. Therefore, merging > patches that fix a bug (like this case) is perfectly okay. Thanks for letting me know, I'm active in too many projects with different definitions in each that I usually try to stay on the safe side :-) But it is good to know and I realize that my usual contributions (which are mostly fixes) would be fine even at that time. Thanks Michal! > Now, there are of course subtle nuances: e.g. merging an aggressive, say > hundred lines of code long bugfix in -rc2 for a theoretical bug is less > desirable. Similarly, a small feature (say a completer to a virsh > command) that's very well understood could be merged if reviewer states > that explicitly "reviewed by and safe for freeze". But of course, > there's no harm merging the feature after the release. It's gotten way > better since we switched to time based releases. Freeze is short and > thus the worst that can happen is the patch is merged after a week. > > #morningCoffeeThoughts > > Michal > -- Christian Ehrhardt Senior Staff Engineer, Ubuntu Server Canonical Ltd
