[libvirt] [PATCH 1/2] add security hook for permitting hugetlbfs access (v2)
From: Serge Hallyn <serge.hallyn@ubuntu.com> When a qemu domain is backed by huge pages, apparmor needs to grant the domain rw access to files under the hugetlbfs mount point. Add a hook, called in qemu_process.c, which ends up adding the read-write access through virt-aa-helper. Qemu will be creating a randomly named file under the mountpoint and unlinking it as soon as it has mmap()d it, therefore we cannot predict the full pathname, but for the same reason it is generally safe to provide access to $path/**. Changelog: v2: use virBuffer* in place of snprintf v2: add test to virt-aa-helper-tests for the virt-aa-helper -F command used. Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 9 +++++++++ src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 10 ++++++++++ src/security/security_manager.h | 3 +++ src/security/security_stack.c | 19 +++++++++++++++++++ tests/virt-aa-helper-test | 3 +++ 7 files changed, 49 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d083e4..cd798a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1074,6 +1074,7 @@ virSecurityManagerSetTapFDLabel; virSecurityManagerStackAddNested; virSecurityManagerVerify; virSecurityManagerGetMountOptions; +virSecurityManagerSetHugepages; # sexpr.h sexpr_append; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ab04599..4418f33 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3482,6 +3482,15 @@ int qemuProcessStart(virConnectPtr conn, } virDomainAuditSecurityLabel(vm, true); + if (driver->hugepage_path && vm->def->mem.hugepage_backed) { + if (virSecurityManagerSetHugepages(driver->securityManager, + vm->def, driver->hugepage_path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set huge path in security driver")); + goto cleanup; + } + } + /* Ensure no historical cgroup for this VM is lying around bogus * settings */ VIR_DEBUG("Ensuring no historical cgroup is lying around"); diff --git a/src/security/security_driver.h b/src/security/security_driver.h index d49b401..ad5097b 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -100,6 +100,9 @@ typedef int (*virSecurityDomainSetTapFDLabel) (virSecurityManagerPtr mgr, int fd); typedef char *(*virSecurityDomainGetMountOptions) (virSecurityManagerPtr mgr, virDomainDefPtr def); +typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path); struct _virSecurityDriver { size_t privateDataLen; @@ -140,6 +143,7 @@ struct _virSecurityDriver { virSecurityDomainSetTapFDLabel domainSetSecurityTapFDLabel; virSecurityDomainGetMountOptions domainGetSecurityMountOptions; + virSecurityDomainSetHugepages domainSetSecurityHugepages; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 0ebd53b..690e4da 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -508,3 +508,13 @@ virSecurityManagerGetNested(virSecurityManagerPtr mgr) list[1] = NULL; return list; } + +int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + if (mgr->drv->domainSetSecurityHugepages) + return mgr->drv->domainSetSecurityHugepages(mgr, vm, path); + + return 0; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 1fdaf8e..2de4d30 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -112,5 +112,8 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm); virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr); +int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr sec, + const char *hugepages_path); #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 1094cbe..c2ccbd0 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -462,6 +462,23 @@ virSecurityStackSetTapFDLabel(virSecurityManagerPtr mgr, return rc; } +static int +virSecurityStackSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetHugepages(item->securityManager, vm, path) < 0) + rc = -1; + } + + return rc; +} + static char *virSecurityStackGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr vm ATTRIBUTE_UNUSED) { return NULL; @@ -529,4 +546,6 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityTapFDLabel = virSecurityStackSetTapFDLabel, .domainGetSecurityMountOptions = virSecurityStackGetMountOptions, + + .domainSetSecurityHugepages = virSecurityStackSetHugepages, }; diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 21a2766..f14db8b 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -316,6 +316,9 @@ testme "0" "initrd is /initrd.img" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,<graphics*,<graphics type='sdl' display=':0.0' xauth='/home/myself/.Xauthority'/>,g" "$template_xml" > "$test_xml" testme "0" "sdl Xauthority" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" "$template_xml" > "$test_xml" +testme "0" "hugepages" "-r -u $valid_uuid -F /run/hugepages/kvm/\*\*" "$test_xml" + testme "0" "help" "-h" echo "" >$output -- 1.7.10.4
From: Serge Hallyn <serge.hallyn@ubuntu.com> When using vnc gaphics over a unix socket, virt-aa-helper needs to provide access for the qemu domain to access the sockfile. Changelog: v2: add testcase to virt-aa-helper-tests to make sure xml with vnc socket works. Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 7 +++++++ tests/virt-aa-helper-test | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e480b30..c6b9903 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1003,6 +1003,13 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.loader, "r") != 0) goto clean; + for (i = 0; i < ctl->def->ngraphics; i++) { + if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + ctl->def->graphics[i]->data.vnc.socket && + vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "rw")) + goto clean; + } + if (ctl->def->ngraphics == 1 && ctl->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) if (vah_add_file(&buf, ctl->def->graphics[0]->data.sdl.xauth, diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index f14db8b..af91c61 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -319,6 +319,9 @@ testme "0" "sdl Xauthority" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" "$template_xml" > "$test_xml" testme "0" "hugepages" "-r -u $valid_uuid -F /run/hugepages/kvm/\*\*" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,<graphics.*>,<graphics type='vnc' socket='/var/lib/libvirt/qemu/myself.vnc'><listen type='address' address='0.0.0.0'/></graphics>,g" "$template_xml" > "$test_xml" +testme "0" "vnc socket" "-r -u $valid_uuid" "$test_xml" + testme "0" "help" "-h" echo "" >$output -- 1.7.10.4
On Tue, Dec 11, 2012 at 08:20:30PM +0000, serge@hallyn.com wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
When using vnc gaphics over a unix socket, virt-aa-helper needs to provide access for the qemu domain to access the sockfile.
Changelog: v2: add testcase to virt-aa-helper-tests to make sure xml with vnc socket works.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 7 +++++++ tests/virt-aa-helper-test | 3 +++ 2 files changed, 10 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e480b30..c6b9903 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1003,6 +1003,13 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.loader, "r") != 0) goto clean;
+ for (i = 0; i < ctl->def->ngraphics; i++) { + if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + ctl->def->graphics[i]->data.vnc.socket && + vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "rw"))
Odd indentation alignment
+ goto clean; + } + if (ctl->def->ngraphics == 1 && ctl->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) if (vah_add_file(&buf, ctl->def->graphics[0]->data.sdl.xauth, diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index f14db8b..af91c61 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -319,6 +319,9 @@ testme "0" "sdl Xauthority" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" "$template_xml" > "$test_xml" testme "0" "hugepages" "-r -u $valid_uuid -F /run/hugepages/kvm/\*\*" "$test_xml"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,<graphics.*>,<graphics type='vnc' socket='/var/lib/libvirt/qemu/myself.vnc'><listen type='address' address='0.0.0.0'/></graphics>,g" "$template_xml" > "$test_xml" +testme "0" "vnc socket" "-r -u $valid_uuid" "$test_xml" + testme "0" "help" "-h"
echo "" >$output
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/11/2012 01:25 PM, Daniel P. Berrange wrote:
On Tue, Dec 11, 2012 at 08:20:30PM +0000, serge@hallyn.com wrote:
Instead of putting '(v2)' as a suffix (which I then have to manually strip via 'git commit --amend'), it is nicer to put it in the prefix [PATCHv2 2/2] (doable with 'git send-email --subject-prefix=PATCHv2', and which gets auto-stripped by 'git am').
From: Serge Hallyn <serge.hallyn@ubuntu.com>
When using vnc gaphics over a unix socket, virt-aa-helper needs to provide access for the qemu domain to access the sockfile.
Changelog: v2: add testcase to virt-aa-helper-tests to make sure xml with vnc socket works.
Likewise, patch changelogs belong best after a --- separator; it's useful for the review, but doesn't need to live in git history.
+ for (i = 0; i < ctl->def->ngraphics; i++) { + if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + ctl->def->graphics[i]->data.vnc.socket && + vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "rw"))
Odd indentation alignment
I fixed the commit messages and this alignment,
ACK
then pushed the series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoting Eric Blake (eblake@redhat.com):
On 12/11/2012 01:25 PM, Daniel P. Berrange wrote:
On Tue, Dec 11, 2012 at 08:20:30PM +0000, serge@hallyn.com wrote:
Instead of putting '(v2)' as a suffix (which I then have to manually strip via 'git commit --amend'), it is nicer to put it in the prefix [PATCHv2 2/2] (doable with 'git send-email --subject-prefix=PATCHv2', and which gets auto-stripped by 'git am').
...
Likewise, patch changelogs belong best after a --- separator; it's useful for the review, but doesn't need to live in git history.
... Sorry (I actually thought that info was wanted in git history). Have noted to make those changes in the future.
I fixed the commit messages and this alignment,
ACK
then pushed the series.
Thanks. -serge
On Tue, Dec 11, 2012 at 08:20:29PM +0000, serge@hallyn.com wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
When a qemu domain is backed by huge pages, apparmor needs to grant the domain rw access to files under the hugetlbfs mount point. Add a hook, called in qemu_process.c, which ends up adding the read-write access through virt-aa-helper. Qemu will be creating a randomly named file under the mountpoint and unlinking it as soon as it has mmap()d it, therefore we cannot predict the full pathname, but for the same reason it is generally safe to provide access to $path/**.
Changelog: v2: use virBuffer* in place of snprintf v2: add test to virt-aa-helper-tests for the virt-aa-helper -F command used.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 9 +++++++++ src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 10 ++++++++++ src/security/security_manager.h | 3 +++ src/security/security_stack.c | 19 +++++++++++++++++++ tests/virt-aa-helper-test | 3 +++ 7 files changed, 49 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d083e4..cd798a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1074,6 +1074,7 @@ virSecurityManagerSetTapFDLabel; virSecurityManagerStackAddNested; virSecurityManagerVerify; virSecurityManagerGetMountOptions; +virSecurityManagerSetHugepages;
# sexpr.h sexpr_append; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ab04599..4418f33 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3482,6 +3482,15 @@ int qemuProcessStart(virConnectPtr conn, } virDomainAuditSecurityLabel(vm, true);
+ if (driver->hugepage_path && vm->def->mem.hugepage_backed) { + if (virSecurityManagerSetHugepages(driver->securityManager, + vm->def, driver->hugepage_path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set huge path in security driver")); + goto cleanup; + } + } + /* Ensure no historical cgroup for this VM is lying around bogus * settings */ VIR_DEBUG("Ensuring no historical cgroup is lying around"); diff --git a/src/security/security_driver.h b/src/security/security_driver.h index d49b401..ad5097b 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -100,6 +100,9 @@ typedef int (*virSecurityDomainSetTapFDLabel) (virSecurityManagerPtr mgr, int fd); typedef char *(*virSecurityDomainGetMountOptions) (virSecurityManagerPtr mgr, virDomainDefPtr def); +typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path);
struct _virSecurityDriver { size_t privateDataLen; @@ -140,6 +143,7 @@ struct _virSecurityDriver { virSecurityDomainSetTapFDLabel domainSetSecurityTapFDLabel;
virSecurityDomainGetMountOptions domainGetSecurityMountOptions; + virSecurityDomainSetHugepages domainSetSecurityHugepages; };
virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 0ebd53b..690e4da 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -508,3 +508,13 @@ virSecurityManagerGetNested(virSecurityManagerPtr mgr) list[1] = NULL; return list; } + +int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + if (mgr->drv->domainSetSecurityHugepages) + return mgr->drv->domainSetSecurityHugepages(mgr, vm, path); + + return 0; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 1fdaf8e..2de4d30 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -112,5 +112,8 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm); virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr); +int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr sec, + const char *hugepages_path);
#endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 1094cbe..c2ccbd0 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -462,6 +462,23 @@ virSecurityStackSetTapFDLabel(virSecurityManagerPtr mgr, return rc; }
+static int +virSecurityStackSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetHugepages(item->securityManager, vm, path) < 0) + rc = -1; + } + + return rc; +} + static char *virSecurityStackGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr vm ATTRIBUTE_UNUSED) { return NULL; @@ -529,4 +546,6 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityTapFDLabel = virSecurityStackSetTapFDLabel,
.domainGetSecurityMountOptions = virSecurityStackGetMountOptions, + + .domainSetSecurityHugepages = virSecurityStackSetHugepages, }; diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 21a2766..f14db8b 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -316,6 +316,9 @@ testme "0" "initrd is /initrd.img" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,<graphics*,<graphics type='sdl' display=':0.0' xauth='/home/myself/.Xauthority'/>,g" "$template_xml" > "$test_xml" testme "0" "sdl Xauthority" "-r -u $valid_uuid" "$test_xml"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" "$template_xml" > "$test_xml" +testme "0" "hugepages" "-r -u $valid_uuid -F /run/hugepages/kvm/\*\*" "$test_xml" + testme "0" "help" "-h"
echo "" >$output --
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Serge Hallyn -
serge＠hallyn.com