9:26 a.m.
Hi,
Currently the hugepages support can automatically detect the hugepages
mount, but it doesn't update the security information. At least for
apparmor we need to be able to add permission for the domain to access
the hugetlbfs mount path.
There are a few ways this could be done,
1. add a virSecuritySetSecurityHugepages or virSecuritySetSecurityHugepagesFD
hook which is called perhaps at qemudStartup
2. optionally add the qemu_driver->hugepage_path to the xml output, at
least for the internal format (which is passed to virt-aa-helper). The
concern I have with this is that it brings up the issue of what to do
when defining a domain which has such an entry.
3. reproduce the logic in virt-aa-helper for detecting the hugepages
mount path. Not preferred obviously.
My guess would be that (1) would be preferred, but I wanted to ask here
first and see if there are other suggestions.
thanks,
-serge
4:48 p.m.
New subject: [libvirt] [PATCH] security hook for hugepages (was Re: virSecurity hook for hugepages?)
Quoting Serge Hallyn (serge.hallyn(a)canonical.com):
Hi,
Currently the hugepages support can automatically detect the hugepages
mount, but it doesn't update the security information. At least for
apparmor we need to be able to add permission for the domain to access
the hugetlbfs mount path.
There are a few ways this could be done,
1. add a virSecuritySetSecurityHugepages or virSecuritySetSecurityHugepagesFD
hook which is called perhaps at qemudStartup
Below is a patch to do (1), defining the hook only for apparmor. I
assume the hook is a noop for selinux, where the whole hugetlbfs can
simply be given a label which libvirt-qemu's domain has access to?
Subject: add security hook for permitting hugetlbfs access
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/**.
Index: libvirt-hugepages/src/qemu/qemu_process.c
===================================================================
--- libvirt-hugepages.orig/src/qemu/qemu_process.c 2012-12-04 22:47:53.366183000 +0000
+++ libvirt-hugepages/src/qemu/qemu_process.c 2012-12-05 22:33:16.955801982 +0000
@@ -3475,6 +3475,15 @@
}
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");
Index: libvirt-hugepages/src/security/security_apparmor.c
===================================================================
--- libvirt-hugepages.orig/src/security/security_apparmor.c 2012-12-04 22:47:53.366183000
+0000
+++ libvirt-hugepages/src/security/security_apparmor.c 2012-12-05 22:34:10.459795263
+0000
@@ -55,6 +55,10 @@
#define SECURITY_APPARMOR_NAME "apparmor"
#define VIRT_AA_HELPER LIBEXECDIR "/virt-aa-helper"
+#ifndef MAX_PATH_LEN
+#define MAX_PATH_LEN 1024
+#endif
+
/* Data structure to pass to *FileIterate so we have everything we need */
struct SDPDOP {
virSecurityManagerPtr mgr;
@@ -840,6 +844,30 @@
}
static int
+ApparmorSetHugepages(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ const char *path)
+{
+ const virSecurityLabelDefPtr secdef =
+ virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
+ int ret;
+ char newpath[MAX_PATH_LEN];
+
+ if (!secdef)
+ return -1;
+
+ if (secdef->imagelabel == NULL)
+ return 0;
+
+ ret = snprintf(newpath, MAX_PATH_LEN, "%s/**", path);
+ if (ret < 0 || ret >= MAX_PATH_LEN) {
+ virReportOOMError();
+ return -1;
+ }
+ return reload_profile(mgr, def, newpath, true);
+}
+
+static int
AppArmorSetFDLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
int fd)
@@ -908,4 +936,6 @@
.domainSetSecurityImageFDLabel = AppArmorSetFDLabel,
.domainSetSecurityTapFDLabel = AppArmorSetFDLabel,
+
+ .domainSetSecurityHugepages = ApparmorSetHugepages,
};
Index: libvirt-hugepages/src/security/security_manager.c
===================================================================
--- libvirt-hugepages.orig/src/security/security_manager.c 2012-12-04 22:47:53.366183000
+0000
+++ libvirt-hugepages/src/security/security_manager.c 2012-12-05 20:30:21.631797074 +0000
@@ -511,3 +511,13 @@
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;
+}
Index: libvirt-hugepages/src/security/security_manager.h
===================================================================
--- libvirt-hugepages.orig/src/security/security_manager.h 2012-12-04 22:47:53.366183000
+0000
+++ libvirt-hugepages/src/security/security_manager.h 2012-12-04 22:47:53.366183000 +0000
@@ -112,5 +112,8 @@
virDomainDefPtr vm);
virSecurityManagerPtr*
virSecurityManagerGetNested(virSecurityManagerPtr mgr);
+int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr,
+ virDomainDefPtr sec,
+ const char *hugepages_path);
#endif /* VIR_SECURITY_MANAGER_H__ */
Index: libvirt-hugepages/src/security/security_driver.h
===================================================================
--- libvirt-hugepages.orig/src/security/security_driver.h 2012-12-04 22:47:53.366183000
+0000
+++ libvirt-hugepages/src/security/security_driver.h 2012-12-05 20:14:34.567796858 +0000
@@ -100,6 +100,9 @@
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 @@
virSecurityDomainSetTapFDLabel domainSetSecurityTapFDLabel;
virSecurityDomainGetMountOptions domainGetSecurityMountOptions;
+ virSecurityDomainSetHugepages domainSetSecurityHugepages;
};
virSecurityDriverPtr virSecurityDriverLookup(const char *name,
Index: libvirt-hugepages/src/libvirt_private.syms
===================================================================
--- libvirt-hugepages.orig/src/libvirt_private.syms 2012-12-04 22:47:53.366183000 +0000
+++ libvirt-hugepages/src/libvirt_private.syms 2012-12-05 17:00:00.127797013 +0000
@@ -1052,6 +1052,7 @@
virSecurityManagerStackAddNested;
virSecurityManagerVerify;
virSecurityManagerGetMountOptions;
+virSecurityManagerSetHugepages;
# sexpr.h
sexpr_append;
Index: libvirt-hugepages/src/security/security_stack.c
===================================================================
--- libvirt-hugepages.orig/src/security/security_stack.c 2012-12-04 22:47:53.366183000
+0000
+++ libvirt-hugepages/src/security/security_stack.c 2012-12-05 20:13:43.675796977 +0000
@@ -462,6 +462,23 @@
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 @@
.domainSetSecurityTapFDLabel = virSecurityStackSetTapFDLabel,
.domainGetSecurityMountOptions = virSecurityStackGetMountOptions,
+
+ .domainSetSecurityHugepages = virSecurityStackSetHugepages,
};
4372
days inactive
4374
days old
1 comments
1 participants
participants (1)
-
Serge Hallyn