Lock all the paths we want to relabel to mutually exclude other
libvirt daemons.

The only culprit here hitch here is that directories can't be
locked. Therefore, when relabeling a directory do not lock it
(this happens only when setting up some domain private paths
anyway, e.g. huge pages directory).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/security/security_selinux.c | 43 +++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index f6416010f9..056637e4cb 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -90,7 +90,7 @@ struct _virSecuritySELinuxContextItem {
 typedef struct _virSecuritySELinuxContextList virSecuritySELinuxContextList;
 typedef virSecuritySELinuxContextList *virSecuritySELinuxContextListPtr;
 struct _virSecuritySELinuxContextList {
-    bool privileged;
+    virSecurityManagerPtr manager;
     virSecuritySELinuxContextItemPtr *items;
     size_t nItems;
 };
@@ -212,8 +212,29 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
                                  void *opaque)
 {
     virSecuritySELinuxContextListPtr list = opaque;
+    bool privileged = virSecurityManagerGetPrivileged(list->manager);
+    const char **paths = NULL;
+    size_t npaths = 0;
     size_t i;
+    int rv;
+    int ret = -1;
 
+    if (VIR_ALLOC_N(paths, list->nItems) < 0)
+        return -1;
+
+    for (i = 0; i < list->nItems; i++) {
+        const char *p = list->items[i]->path;
+
+        if (virFileIsDir(p))
+            continue;
+
+        VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p);
+    }
+
+    if (virSecurityManagerMetadataLock(list->manager, paths, npaths) < 0)
+        goto cleanup;
+
+    rv = 0;
     for (i = 0; i < list->nItems; i++) {
         virSecuritySELinuxContextItemPtr item = list->items[i];
 
@@ -221,11 +242,22 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
         if (virSecuritySELinuxSetFileconHelper(item->path,
                                                item->tcon,
                                                item->optional,
-                                               list->privileged) < 0)
-            return -1;
+                                               privileged) < 0) {
+            rv = -1;
+            break;
+        }
     }
 
-    return 0;
+    if (virSecurityManagerMetadataUnlock(list->manager, paths, npaths) < 0)
+        goto cleanup;
+
+    if (rv < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(paths);
+    return ret;
 }
 
 
@@ -1010,7 +1042,6 @@ virSecuritySELinuxGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
 static int
 virSecuritySELinuxTransactionStart(virSecurityManagerPtr mgr)
 {
-    bool privileged = virSecurityManagerGetPrivileged(mgr);
     virSecuritySELinuxContextListPtr list;
 
     list = virThreadLocalGet(&contextList);
@@ -1023,7 +1054,7 @@ virSecuritySELinuxTransactionStart(virSecurityManagerPtr mgr)
     if (VIR_ALLOC(list) < 0)
         return -1;
 
-    list->privileged = privileged;
+    list->manager = mgr;
 
     if (virThreadLocalSet(&contextList, list) < 0) {
         virReportSystemError(errno, "%s",
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

$SUBJ

s/dac/selinux

On 09/10/2018 05:36 AM, Michal Privoznik wrote:
> Lock all the paths we want to relabel to mutually exclude other
> libvirt daemons.
> 
> The only culprit here hitch here is that directories can't be

Where have I seen this before?

> locked. Therefore, when relabeling a directory do not lock it
> (this happens only when setting up some domain private paths
> anyway, e.g. huge pages directory).
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/security/security_selinux.c | 43 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 

I shall say "similar comments to my DAC review" (ref/unref, more
comments in TransactionRun, and if you want use rv = *SetFilecon* and if
(rv < 0) break...

And, then you can apply the

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list