Testing qemu-2.10-rc2 shows issues like:
  qemu-system-x86_64: -drive file=/var/lib/uvtool/libvirt/images/kvmguest- \
  artful-normal.qcow,format=qcow2,if=none,id=drive-virtio-disk0:
  Failed to lock byte 100

It seems the following qemu commit changed the needs for the backing
image rules:

(qemu) commit 244a5668106297378391b768e7288eb157616f64
Author: Fam Zheng <famz@redhat.com>
    file-posix: Add image locking to perm operations

The block appears as:
 apparmor="DENIED" operation="file_lock" [...]
 name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
 [...] comm="qemu-system-x86" requested_mask="k" denied_mask="k"

With that qemu change in place the rules generated for the image
and backing files need the allowance to also lock (k) the files.

Disks are added via add_file_path and with this fix rules now get
that permission, but no other rules are changed, example:
  -  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rw,
  +  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rwk

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

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35dcb35..ab82f12 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -892,11 +892,11 @@ add_file_path(virDomainDiskDefPtr disk,
 
     if (depth == 0) {
         if (disk->src->readonly)
-            ret = vah_add_file(buf, path, "r");
+            ret = vah_add_file(buf, path, "rk");
         else
-            ret = vah_add_file(buf, path, "rw");
+            ret = vah_add_file(buf, path, "rwk");
     } else {
-        ret = vah_add_file(buf, path, "r");
+        ret = vah_add_file(buf, path, "rk");
     }
 
     if (ret != 0)
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Aug 10, 2017 at 11:19 AM, Christian Ehrhardt <
christian.ehrhardt@canonical.com> wrote:

> Testing qemu-2.10-rc2 shows issues like:
>   qemu-system-x86_64: -drive file=/var/lib/uvtool/libvirt/images/kvmguest-
> \
>   artful-normal.qcow,format=qcow2,if=none,id=drive-virtio-disk0:
>   Failed to lock byte 100
>
> It seems the following qemu commit changed the needs for the backing
> image rules:
>
> (qemu) commit 244a5668106297378391b768e7288eb157616f64
> Author: Fam Zheng <famz@redhat.com>
>     file-posix: Add image locking to perm operations
>
>
Additional Note (but not part of the patch description):
I thought at first I'd be on the "old kernel but new glibc" case of [1] in
qemu.
But as explained in the patch it turned out that instead my kernel was new
enough and instead I ran into apparmor denials.

Even if that change would be accepted in qemu we would still need the
apparmor fix I proposed here, but it is valid context that people might be
interested in.

[1]: https://lists.gnu.org/archive/html/qemu-block/2017-07/msg01294.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Hi,

Christian Ehrhardt:
> With that qemu change in place the rules generated for the image
> and backing files need the allowance to also lock (k) the files.

I'm not a C developer so I cannot review the proposed patch, but FWIW
Christian's reasoning makes sense to me and from a high-level AppArmor
perspective, the proposed change seems entirely harmless.

Cheers,
-- 
intrigeri

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Ping - opinions on this or is it ready to be committed?

On this reply setting Guido on CC as he has experience on apparmor patches
in libvirt and commit rights.


On Fri, Aug 11, 2017 at 8:58 PM, intrigeri <intrigeri+libvirt@boum.org>
wrote:

> Hi,
>
> Christian Ehrhardt:
> > With that qemu change in place the rules generated for the image
> > and backing files need the allowance to also lock (k) the files.
>
> I'm not a C developer so I cannot review the proposed patch, but FWIW
> Christian's reasoning makes sense to me and from a high-level AppArmor
> perspective, the proposed change seems entirely harmless.
>
> Cheers,
> --
> intrigeri
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>



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

On 08/10/2017 11:19 AM, Christian Ehrhardt wrote:
> Testing qemu-2.10-rc2 shows issues like:
>   qemu-system-x86_64: -drive file=/var/lib/uvtool/libvirt/images/kvmguest- \
>   artful-normal.qcow,format=qcow2,if=none,id=drive-virtio-disk0:
>   Failed to lock byte 100
> 
> It seems the following qemu commit changed the needs for the backing
> image rules:
> 
> (qemu) commit 244a5668106297378391b768e7288eb157616f64
> Author: Fam Zheng <famz@redhat.com>
>     file-posix: Add image locking to perm operations
> 
> The block appears as:
>  apparmor="DENIED" operation="file_lock" [...]
>  name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
>  [...] comm="qemu-system-x86" requested_mask="k" denied_mask="k"
> 
> With that qemu change in place the rules generated for the image
> and backing files need the allowance to also lock (k) the files.
> 
> Disks are added via add_file_path and with this fix rules now get
> that permission, but no other rules are changed, example:
>   -  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rw,
>   +  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rwk
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  src/security/virt-aa-helper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list