[libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10

Christian Ehrhardt posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1502960143-3699-1-git-send-email-christian.ehrhardt@canonical.com
src/security/virt-aa-helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10
Posted by Christian Ehrhardt 6 years, 8 months ago
Testing qemu-2.10-rc3 shows issues like:
  qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/
  7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100

There is an apparmor deny due to qemu now locking those files:
 apparmor="DENIED" operation="file_lock" [...]
 name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd"
 name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
 [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k"

The profile needs to allow locking for loader and nvram files via
the locking (k) rule.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 src/security/virt-aa-helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index ab82f12..2308323 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1029,11 +1029,11 @@ get_files(vahControl * ctl)
             goto cleanup;
 
     if (ctl->def->os.loader && ctl->def->os.loader->path)
-        if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0)
+        if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0)
             goto cleanup;
 
     if (ctl->def->os.loader && ctl->def->os.loader->nvram)
-        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rw") != 0)
+        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0)
             goto cleanup;
 
     for (i = 0; i < ctl->def->ngraphics; i++) {
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10
Posted by Michal Privoznik 6 years, 8 months ago
On 08/17/2017 10:55 AM, Christian Ehrhardt wrote:
> Testing qemu-2.10-rc3 shows issues like:
>   qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/
>   7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100
> 
> There is an apparmor deny due to qemu now locking those files:
>  apparmor="DENIED" operation="file_lock" [...]
>  name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd"
>  name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
>  [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k"
> 
> The profile needs to allow locking for loader and nvram files via
> the locking (k) rule.
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  src/security/virt-aa-helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index ab82f12..2308323 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1029,11 +1029,11 @@ get_files(vahControl * ctl)
>              goto cleanup;
>  
>      if (ctl->def->os.loader && ctl->def->os.loader->path)
> -        if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0)
> +        if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0)
>              goto cleanup;
>  
>      if (ctl->def->os.loader && ctl->def->os.loader->nvram)
> -        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rw") != 0)
> +        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0)
>              goto cleanup;
>  
>      for (i = 0; i < ctl->def->ngraphics; i++) {
> 

Is this result of qemu image locking? I'm not against this patch just
asking.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10
Posted by Christian Ehrhardt 6 years, 8 months ago
Hi Michal,
yes it is essentially a "follow on" or "another case of" what I already
submitted in
https://www.redhat.com/archives/libvir-list/2017-August/msg00312.html

I didn't want to add walls of text each time, but yeah due to image locking.
Let me know if I should submit a v2 which states so more explicitly.


On Thu, Aug 17, 2017 at 1:23 PM, Michal Privoznik <mprivozn@redhat.com>
wrote:

> On 08/17/2017 10:55 AM, Christian Ehrhardt wrote:
> > Testing qemu-2.10-rc3 shows issues like:
> >   qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/
> >   7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100
> >
> > There is an apparmor deny due to qemu now locking those files:
> >  apparmor="DENIED" operation="file_lock" [...]
> >  name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd"
> >  name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
> >  [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k"
> >
> > The profile needs to allow locking for loader and nvram files via
> > the locking (k) rule.
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> >  src/security/virt-aa-helper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/security/virt-aa-helper.c
> b/src/security/virt-aa-helper.c
> > index ab82f12..2308323 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -1029,11 +1029,11 @@ get_files(vahControl * ctl)
> >              goto cleanup;
> >
> >      if (ctl->def->os.loader && ctl->def->os.loader->path)
> > -        if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0)
> > +        if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0)
> >              goto cleanup;
> >
> >      if (ctl->def->os.loader && ctl->def->os.loader->nvram)
> > -        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rw") != 0)
> > +        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0)
> >              goto cleanup;
> >
> >      for (i = 0; i < ctl->def->ngraphics; i++) {
> >
>
> Is this result of qemu image locking? I'm not against this patch just
> asking.
>
> Michal
>



-- 
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] virt-aa-helper: locking loader/nvram for qemu 2.10
Posted by Michal Privoznik 6 years, 8 months ago
On 08/17/2017 02:03 PM, Christian Ehrhardt wrote:
> Hi Michal,
> yes it is essentially a "follow on" or "another case of" what I already
> submitted in
> https://www.redhat.com/archives/libvir-list/2017-August/msg00312.html
> 
> I didn't want to add walls of text each time, but yeah due to image locking.
> Let me know if I should submit a v2 which states so more explicitly.

No need. This looks reasonable. Even though we might want to handle that
qemu feature in libvirt somehow, this can go in meanwhile.

ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10
Posted by Jamie Strandboge 6 years, 8 months ago
On Thu, 2017-08-17 at 10:55 +0200, Christian Ehrhardt wrote:
> Testing qemu-2.10-rc3 shows issues like:
>   qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/
>   7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100
> 
> There is an apparmor deny due to qemu now locking those files:
>  apparmor="DENIED" operation="file_lock" [...]
>  name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd"
>  name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
>  [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k"
> 
> The profile needs to allow locking for loader and nvram files via
> the locking (k) rule.
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  src/security/virt-aa-helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index ab82f12..2308323 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1029,11 +1029,11 @@ get_files(vahControl * ctl)
>              goto cleanup;
>  
>      if (ctl->def->os.loader && ctl->def->os.loader->path)
> -        if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0)
> +        if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0)
>              goto cleanup;
>  
>      if (ctl->def->os.loader && ctl->def->os.loader->nvram)
> -        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rw") != 0)
> +        if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0)
>              goto cleanup;
>  
>      for (i = 0; i < ctl->def->ngraphics; i++) {

This LGTM. +1
-- 
Jamie Strandboge             | http://www.canonical.com--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list