Re: [libvirt] [PATCH 1/2] add security hook for permitting hugetlbfs access (v2)

From: Serge Hallyn <serge.hallyn(a)ubuntu.com&gt; 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(a)ubuntu.com&gt; --- 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 --