Re: [libvirt] [PATCH v3 12/17] security_selinux: Label host side of NVDIMM
On 03/09/2017 11:06 AM, Michal Privoznik wrote:
When domain is being started up, we ought to relabel the host
side of NVDIMM so qemu has access to it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/security/security_selinux.c | 66 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e22de0653..1be2acd27 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1381,6 +1381,62 @@ virSecuritySELinuxRestoreInputLabel(virSecurityManagerPtr mgr,
}
+static int
+virSecuritySELinuxSetMemoryLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+ virSecurityLabelDefPtr seclabel;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (seclabel == NULL)
+ return 0;
Since it doesn't matter for DIMM, should this go in the NVDIMM label?
Although I do see this follow a couple of the other Set*Label functions
when there's only one of the switch case statements that uses the
seclabel. I guess for consistency it can stay as is, although I
wouldn't object to altering code for those single switch/case conditions
Also I note that the security_dac code looks at the "->relabel" when
making a decision, but that's not done here.
+
+ switch ((virDomainMemoryModel) mem->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ if (virSecuritySELinuxSetFilecon(mgr, mem->nvdimmPath,
+ seclabel->imagelabel) < 0)
+ return -1;
+ break;
+
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
+static int
+virSecuritySELinuxRestoreMemoryLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+ int ret = -1;
+ virSecurityLabelDefPtr seclabel;
+
+ switch ((virDomainMemoryModel) mem->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (seclabel == NULL)
+ return 0;
Ironically you did change this one to be different... Similar comment
regarding the relabel
Conditional ACK of course depending on the relabel thing - you could
explain or just provide something that you'll squash in....
John
+
+ ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->nvdimmPath);
+ break;
+
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ ret = 0;
+ break;
+ }
+
+ return ret;
+}
+
+
static int
virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
@@ -2325,6 +2381,11 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr,
rc = -1;
}
+ for (i = 0; i < def->nmems; i++) {
+ if (virSecuritySELinuxRestoreMemoryLabel(mgr, def, def->mems[i]) < 0)
+ return -1;
+ }
+
for (i = 0; i < def->ndisks; i++) {
virDomainDiskDefPtr disk = def->disks[i];
@@ -2711,6 +2772,11 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
return -1;
}
+ for (i = 0; i < def->nmems; i++) {
+ if (virSecuritySELinuxSetMemoryLabel(mgr, def, def->mems[i]) < 0)
+ return -1;
+ }
+
if (def->tpm) {
if (virSecuritySELinuxSetTPMFileLabel(mgr, def, def->tpm) < 0)
return -1;