Our decision whether to remember seclabel for a disk image depends on a few factors. If the image is readonly or shared or not top parent of a backing chain the remembering is suppressed for the image. However, the virSecurityManagerSetImageLabel() is too low level to determine whether passed @src is top parent or not. Even though it has domain definition available, in some cases (like snapshots or block copy) the @src is added to the definition only after the operation succeeded. Therefore, introduce a flag which callers can use to help us with the decision. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/security/security_dac.c | 16 +++++++++++----- src/security/security_manager.h | 1 + src/security/security_selinux.c | 18 ++++++++++++------ 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index f412054d0e..3f8b04b307 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -889,14 +889,14 @@ static int virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src, - virStorageSourcePtr parent) + virStorageSourcePtr parent, + bool is_topparent) { virSecurityLabelDefPtr secdef; virSecurityDeviceLabelDefPtr disk_seclabel; virSecurityDeviceLabelDefPtr parent_seclabel = NULL; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); bool remember; - bool is_toplevel = parent == src || parent->externalDataStore == src; uid_t user; gid_t group; @@ -954,7 +954,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, * but the top layer, or read only image, or disk explicitly * marked as shared. */ - remember = is_toplevel && !src->readonly && !src->shared; + remember = is_topparent && !src->readonly && !src->shared; return virSecurityDACSetOwnership(mgr, src, NULL, user, group, remember); } @@ -967,10 +967,13 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr, virStorageSourcePtr parent, virSecurityDomainImageLabelFlags flags) { + bool is_topparent = flags & VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT; virStorageSourcePtr n; + flags &= ~VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT; + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent) < 0) + if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent, is_topparent) < 0) return -1; if (n->externalDataStore && @@ -983,6 +986,8 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr, if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) break; + + is_topparent = false; } return 0; @@ -2114,7 +2119,8 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR) continue; if (virSecurityDACSetImageLabel(mgr, def, def->disks[i]->src, - VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0) + VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | + VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT) < 0) return -1; } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index b92ea5dc87..11904fda89 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -151,6 +151,7 @@ virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr); typedef enum { VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN = 1 << 0, + VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT = 1 << 1, } virSecurityDomainImageLabelFlags; int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 2241a35e6e..0aa0c2bb71 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1824,7 +1824,8 @@ static int virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src, - virStorageSourcePtr parent) + virStorageSourcePtr parent, + bool is_topparent) { virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDefPtr secdef; @@ -1832,7 +1833,6 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, virSecurityDeviceLabelDefPtr parent_seclabel = NULL; char *use_label = NULL; bool remember; - bool is_toplevel = parent == src || parent->externalDataStore == src; g_autofree char *vfioGroupDev = NULL; const char *path = src->path; int ret; @@ -1856,7 +1856,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, * but the top layer, or read only image, or disk explicitly * marked as shared. */ - remember = is_toplevel && !src->readonly && !src->shared; + remember = is_topparent && !src->readonly && !src->shared; disk_seclabel = virStorageSourceGetSecurityLabelDef(src, SECURITY_SELINUX_NAME); @@ -1873,7 +1873,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, return 0; use_label = parent_seclabel->label; - } else if (is_toplevel) { + } else if (parent == src || parent->externalDataStore == src) { if (src->shared) { use_label = data->file_context; } else if (src->readonly) { @@ -1927,10 +1927,13 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr, virStorageSourcePtr parent, virSecurityDomainImageLabelFlags flags) { + bool is_topparent = flags & VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT; virStorageSourcePtr n; + flags &= ~VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT; + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent) < 0) + if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent, is_topparent) < 0) return -1; if (n->externalDataStore && @@ -1943,6 +1946,8 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr, if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) break; + + is_topparent = false; } return 0; @@ -3146,7 +3151,8 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr, continue; } if (virSecuritySELinuxSetImageLabel(mgr, def, def->disks[i]->src, - VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0) + VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | + VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT) < 0) return -1; } /* XXX fixme process def->fss if relabel == true */ -- 2.24.1