Re: [libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
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 > 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 > --- > 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
Re: [libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
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 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
Re: [libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
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
[libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
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 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 --- 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
Re: [libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
On Thu, Aug 10, 2017 at 11:19 AM, Christian Ehrhardt < christian.ehrha...@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 > 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