[libvirt] [PATCH] apparmor: fix vfio usage without initial hostdev

Christian Ehrhardt posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180611115254.18111-1-christian.ehrhardt@canonical.com
Test syntax-check passed
examples/apparmor/libvirt-qemu | 3 +++
1 file changed, 3 insertions(+)
[libvirt] [PATCH] apparmor: fix vfio usage without initial hostdev
Posted by Christian Ehrhardt 5 years, 9 months ago
The base vfio has not much functionality but to provide a custom
container by opening this path.
See https://www.kernel.org/doc/Documentation/vfio.txt for more.

Systems with static hostdevs will get /dev/vfio/vfio by virt-aa-hotplug
right from the beginning. But if the guest initially had no hostdev at
all it will run into the following deny before the security module
labelling callbacks will make the actual vfio device (like /dev/vfio/93)
known.

Access by qemu is "wr" even thou in theory it could maybe be "r":
[ 2652.756712] audit: type=1400 audit(1491303691.719:25):
  apparmor="DENIED" operation="open"
  profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a"
  name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"
  requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1775777

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 examples/apparmor/libvirt-qemu | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 2c47652250..874aca2092 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -193,6 +193,9 @@
   deny /dev/shm/lttng-ust-wait-* r,
   deny /run/shm/lttng-ust-wait-* r,
 
+  # for vfio hotplug on systems without static vfio (LP: #1775777)
+  /dev/vfio/vfio rw,
+
   # required for sasl GSSAPI plugin
   /etc/gss/mech.d/ r,
   /etc/gss/mech.d/* r,
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] apparmor: fix vfio usage without initial hostdev
Posted by Erik Skultety 5 years, 9 months ago
On Mon, Jun 11, 2018 at 01:52:54PM +0200, Christian Ehrhardt wrote:
> The base vfio has not much functionality but to provide a custom
> container by opening this path.
> See https://www.kernel.org/doc/Documentation/vfio.txt for more.
>
> Systems with static hostdevs will get /dev/vfio/vfio by virt-aa-hotplug
> right from the beginning. But if the guest initially had no hostdev at
> all it will run into the following deny before the security module
> labelling callbacks will make the actual vfio device (like /dev/vfio/93)
> known.
>
> Access by qemu is "wr" even thou in theory it could maybe be "r":

^this sentence is not necessary for the commit message and should be dropped.

> [ 2652.756712] audit: type=1400 audit(1491303691.719:25):
>   apparmor="DENIED" operation="open"
>   profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a"
>   name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"
>   requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
>
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1775777
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  examples/apparmor/libvirt-qemu | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index 2c47652250..874aca2092 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -193,6 +193,9 @@
>    deny /dev/shm/lttng-ust-wait-* r,
>    deny /run/shm/lttng-ust-wait-* r,
>
> +  # for vfio hotplug on systems without static vfio (LP: #1775777)
> +  /dev/vfio/vfio rw,

Per the kernel doc you linked in the commit message I think this is fine.

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] apparmor: fix vfio usage without initial hostdev
Posted by Christian Ehrhardt 5 years, 9 months ago
On Mon, Jun 11, 2018 at 2:48 PM, Erik Skultety <eskultet@redhat.com> wrote:

> On Mon, Jun 11, 2018 at 01:52:54PM +0200, Christian Ehrhardt wrote:
> > The base vfio has not much functionality but to provide a custom
> > container by opening this path.
> > See https://www.kernel.org/doc/Documentation/vfio.txt for more.
> >
> > Systems with static hostdevs will get /dev/vfio/vfio by virt-aa-hotplug
> > right from the beginning. But if the guest initially had no hostdev at
> > all it will run into the following deny before the security module
> > labelling callbacks will make the actual vfio device (like /dev/vfio/93)
> > known.
> >
> > Access by qemu is "wr" even thou in theory it could maybe be "r":
>
> ^this sentence is not necessary for the commit message and should be
> dropped.
>

Agreed will be dropped on a V2 or when pushing the change.


> > [ 2652.756712] audit: type=1400 audit(1491303691.719:25):
> >   apparmor="DENIED" operation="open"
> >   profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a"
> >   name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"
> >   requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
> >
> > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
> > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1775777
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> > ---
> >  examples/apparmor/libvirt-qemu | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-
> qemu
> > index 2c47652250..874aca2092 100644
> > --- a/examples/apparmor/libvirt-qemu
> > +++ b/examples/apparmor/libvirt-qemu
> > @@ -193,6 +193,9 @@
> >    deny /dev/shm/lttng-ust-wait-* r,
> >    deny /run/shm/lttng-ust-wait-* r,
> >
> > +  # for vfio hotplug on systems without static vfio (LP: #1775777)
> > +  /dev/vfio/vfio rw,
>
> Per the kernel doc you linked in the commit message I think this is fine.
>
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
>

Thanks, will add the reviewed-by on a V2 or when pushing the change.

I'll leave it out for discussion as-is a bit more ...



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] apparmor: fix vfio usage without initial hostdev
Posted by Jamie Strandboge 5 years, 9 months ago
On Mon, 2018-06-11 at 13:52 +0200, Christian Ehrhardt wrote:
> The base vfio has not much functionality but to provide a custom
> container by opening this path.
> See https://www.kernel.org/doc/Documentation/vfio.txt for more.
> 
> Systems with static hostdevs will get /dev/vfio/vfio by virt-aa-
> hotplug
> right from the beginning. But if the guest initially had no hostdev
> at
> all it will run into the following deny before the security module
> labelling callbacks will make the actual vfio device (like
> /dev/vfio/93)
> known.
> 
> Access by qemu is "wr" even thou in theory it could maybe be "r":
> [ 2652.756712] audit: type=1400 audit(1491303691.719:25):
>   apparmor="DENIED" operation="open"
>   profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a"
>   name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"
>   requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
> 
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1775777
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  examples/apparmor/libvirt-qemu | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/examples/apparmor/libvirt-qemu
> b/examples/apparmor/libvirt-qemu
> index 2c47652250..874aca2092 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -193,6 +193,9 @@
>    deny /dev/shm/lttng-ust-wait-* r,
>    deny /run/shm/lttng-ust-wait-* r,
>  
> +  # for vfio hotplug on systems without static vfio (LP: #1775777)
> +  /dev/vfio/vfio rw,
> +

Makes sense. If the guest doesn't start with vfio then libvirtd is
going to have to give it to the guest and so libvirtd would need this
access. +1 to apply.

-- 
Jamie Strandboge             | http://www.canonical.com--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] apparmor: fix vfio usage without initial hostdev
Posted by Christian Ehrhardt 5 years, 9 months ago
Thanks everybody,
pushed with the commit message cleaned up and acks/reviews added.

On Tue, Jun 12, 2018 at 2:39 PM, Jamie Strandboge <jamie@canonical.com>
wrote:

> On Mon, 2018-06-11 at 13:52 +0200, Christian Ehrhardt wrote:
> > The base vfio has not much functionality but to provide a custom
> > container by opening this path.
> > See https://www.kernel.org/doc/Documentation/vfio.txt for more.
> >
> > Systems with static hostdevs will get /dev/vfio/vfio by virt-aa-
> > hotplug
> > right from the beginning. But if the guest initially had no hostdev
> > at
> > all it will run into the following deny before the security module
> > labelling callbacks will make the actual vfio device (like
> > /dev/vfio/93)
> > known.
> >
> > Access by qemu is "wr" even thou in theory it could maybe be "r":
> > [ 2652.756712] audit: type=1400 audit(1491303691.719:25):
> >   apparmor="DENIED" operation="open"
> >   profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a"
> >   name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86"
> >   requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
> >
> > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
> > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1775777
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> > ---
> >  examples/apparmor/libvirt-qemu | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/examples/apparmor/libvirt-qemu
> > b/examples/apparmor/libvirt-qemu
> > index 2c47652250..874aca2092 100644
> > --- a/examples/apparmor/libvirt-qemu
> > +++ b/examples/apparmor/libvirt-qemu
> > @@ -193,6 +193,9 @@
> >    deny /dev/shm/lttng-ust-wait-* r,
> >    deny /run/shm/lttng-ust-wait-* r,
> >
> > +  # for vfio hotplug on systems without static vfio (LP: #1775777)
> > +  /dev/vfio/vfio rw,
> > +
>
> Makes sense. If the guest doesn't start with vfio then libvirtd is
> going to have to give it to the guest and so libvirtd would need this
> access. +1 to apply.
>
> --
> Jamie Strandboge             | http://www.canonical.com




-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list