[PATCH] apparmor: allow getattr on usb devices

Christian Ehrhardt christian.ehrhardt at canonical.com
Tue Nov 22 09:50:59 UTC 2022


On Tue, Nov 22, 2022 at 9:55 AM Michal Prívozník <mprivozn at redhat.com> wrote:
>
> On 11/22/22 09:47, Christian Ehrhardt wrote:
> > On Mon, Nov 21, 2022 at 4:51 PM Michal Prívozník <mprivozn at redhat.com> wrote:
> >>
> >> On 11/17/22 09:42, christian.ehrhardt at canonical.com wrote:
> >>> From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> >>>
> >>> 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 at canonical.com>
> >>> ---
> >>>  src/security/apparmor/libvirt-qemu | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>
> >> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> >
> > 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



More information about the libvir-list mailing list