Similarly to what I did in DAC driver, this also requires the
same SELinux label to be used for shared paths. If a path is
already in use by a domain (or domains) then and the domain we
are starting now wants to access the path it has to have the same
SELinux label. This might look too restrictive as the new label
can still guarantee access to already running domains but in
reality it is very unlikely and usually an admin mistake.

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

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 715d9a428b..43d2ef32a5 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -33,6 +33,7 @@
 
 #include "security_driver.h"
 #include "security_selinux.h"
+#include "security_util.h"
 #include "virerror.h"
 #include "viralloc.h"
 #include "virlog.h"
@@ -197,14 +198,40 @@ virSecuritySELinuxTransactionAppend(const char *path,
 }
 
 
+static int
+virSecuritySELinuxRememberLabel(const char *path,
+                                const security_context_t con)
+{
+    return virSecuritySetRememberedLabel(SECURITY_SELINUX_NAME,
+                                         path, con);
+}
+
+
+static int
+virSecuritySELinuxRecallLabel(const char *path,
+                              security_context_t *con)
+{
+    if (virSecurityGetRememberedLabel(SECURITY_SELINUX_NAME,
+                                      path, con) < 0)
+        return -1;
+
+    if (!con)
+        return 1;
+
+    return 0;
+}
+
+
 static int virSecuritySELinuxSetFileconHelper(const char *path,
                                               const char *tcon,
                                               bool optional,
-                                              bool privileged);
+                                              bool privileged,
+                                              bool remember);
 
 
 static int virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
-                                              const char *path);
+                                              const char *path,
+                                              bool recall);
 
 
 /**
@@ -255,10 +282,12 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
             rv = virSecuritySELinuxSetFileconHelper(item->path,
                                                     item->tcon,
                                                     item->optional,
-                                                    privileged);
+                                                    privileged,
+                                                    list->lock);
         } else {
             rv = virSecuritySELinuxRestoreFileLabel(list->manager,
-                                                    item->path);
+                                                    item->path,
+                                                    list->lock);
         }
 
         if (rv < 0)
@@ -1275,16 +1304,54 @@ virSecuritySELinuxSetFileconImpl(const char *path, const char *tcon,
 
 static int
 virSecuritySELinuxSetFileconHelper(const char *path, const char *tcon,
-                                   bool optional, bool privileged)
+                                   bool optional, bool privileged, bool remember)
 {
+    security_context_t econ = NULL;
+    int refcount;
     int rc;
+    int ret = -1;
 
     if ((rc = virSecuritySELinuxTransactionAppend(path, tcon, optional, false)) < 0)
         return -1;
     else if (rc > 0)
         return 0;
 
-    return virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged);
+    if (remember) {
+        if (getfilecon_raw(path, &econ) < 0 &&
+            errno != ENOTSUP && errno != ENODATA) {
+            virReportSystemError(errno,
+                                 _("unable to get SELinux context of %s"),
+                                 path);
+            goto cleanup;
+        }
+
+        if (econ) {
+            refcount = virSecuritySELinuxRememberLabel(path, econ);
+            if (refcount < 0) {
+                goto cleanup;
+            } else if (refcount > 1) {
+                /* Refcount is greater than 1 which means that there
+                 * is @refcount domains using the @path. Do not
+                 * change the label (as it would almost certainly
+                 * cause the other domains to lose access to the
+                 * @path). */
+                if (STRNEQ(econ, tcon)) {
+                    virReportError(VIR_ERR_OPERATION_INVALID,
+                                   _("Setting different SELinux label on %s "
+                                     "which is already in use"), path);
+                    goto cleanup;
+                }
+            }
+        }
+    }
+
+    if (virSecuritySELinuxSetFileconImpl(path, tcon, optional, privileged) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    freecon(econ);
+    return ret;
 }
 
 
@@ -1293,7 +1360,7 @@ virSecuritySELinuxSetFileconOptional(virSecurityManagerPtr mgr,
                                      const char *path, const char *tcon)
 {
     bool privileged = virSecurityManagerGetPrivileged(mgr);
-    return virSecuritySELinuxSetFileconHelper(path, tcon, true, privileged);
+    return virSecuritySELinuxSetFileconHelper(path, tcon, true, privileged, false);
 }
 
 static int
@@ -1301,7 +1368,7 @@ virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
                              const char *path, const char *tcon)
 {
     bool privileged = virSecurityManagerGetPrivileged(mgr);
-    return virSecuritySELinuxSetFileconHelper(path, tcon, false, privileged);
+    return virSecuritySELinuxSetFileconHelper(path, tcon, false, privileged, false);
 }
 
 static int
@@ -1362,7 +1429,8 @@ getContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
  * errors that the caller(s) are already dealing with */
 static int
 virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
-                                   const char *path)
+                                   const char *path,
+                                   bool recall)
 {
     bool privileged = virSecurityManagerGetPrivileged(mgr);
     struct stat buf;
@@ -1386,26 +1454,35 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
         goto cleanup;
     }
 
-    if (stat(newpath, &buf) != 0) {
-        VIR_WARN("cannot stat %s: %s", newpath,
-                 virStrerror(errno, ebuf, sizeof(ebuf)));
-        goto cleanup;
-    }
-
-    if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
-        /* Any user created path likely does not have a default label,
-         * which makes this an expected non error
-         */
-        VIR_WARN("cannot lookup default selinux label for %s", newpath);
-        ret = 0;
-        goto cleanup;
-    }
-
-    if ((rc = virSecuritySELinuxTransactionAppend(path, fcon, false, true)) < 0)
+    if ((rc = virSecuritySELinuxTransactionAppend(path, NULL, false, true)) < 0)
         return -1;
     else if (rc > 0)
         return 0;
 
+    if (recall) {
+        if ((rc = virSecuritySELinuxRecallLabel(newpath, &fcon)) < 0) {
+            goto cleanup;
+        } else if (rc > 0) {
+            ret = 0;
+            goto cleanup;
+        }
+    } else {
+        if (stat(newpath, &buf) != 0) {
+            VIR_WARN("cannot stat %s: %s", newpath,
+                     virStrerror(errno, ebuf, sizeof(ebuf)));
+            goto cleanup;
+        }
+
+        if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
+            /* Any user created path likely does not have a default label,
+             * which makes this an expected non error
+             */
+            VIR_WARN("cannot lookup default selinux label for %s", newpath);
+            ret = 0;
+            goto cleanup;
+        }
+    }
+
     if (virSecuritySELinuxSetFileconImpl(newpath, fcon, false, privileged) < 0)
         goto cleanup;
 
@@ -1460,7 +1537,7 @@ virSecuritySELinuxRestoreInputLabel(virSecurityManagerPtr mgr,
 
     switch ((virDomainInputType)input->type) {
     case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
-        rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev);
+        rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev, false);
         break;
 
     case VIR_DOMAIN_INPUT_TYPE_MOUSE:
@@ -1516,7 +1593,7 @@ virSecuritySELinuxRestoreMemoryLabel(virSecurityManagerPtr mgr,
         if (!seclabel || !seclabel->relabel)
             return 0;
 
-        ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->nvdimmPath);
+        ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->nvdimmPath, false);
         break;
 
     case VIR_DOMAIN_MEMORY_MODEL_DIMM:
@@ -1595,10 +1672,10 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr,
     switch (tpm->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
         tpmdev = tpm->data.passthrough.source.data.file.path;
-        rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev);
+        rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev, false);
 
         if ((cancel_path = virTPMCreateCancelPath(tpmdev)) != NULL) {
-            if (virSecuritySELinuxRestoreFileLabel(mgr, cancel_path) < 0)
+            if (virSecuritySELinuxRestoreFileLabel(mgr, cancel_path, false) < 0)
                 rc = -1;
             VIR_FREE(cancel_path);
         }
@@ -1665,7 +1742,7 @@ virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr,
         }
     }
 
-    return virSecuritySELinuxRestoreFileLabel(mgr, src->path);
+    return virSecuritySELinuxRestoreFileLabel(mgr, src->path, false);
 }
 
 
@@ -2053,7 +2130,7 @@ virSecuritySELinuxRestorePCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED,
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecuritySELinuxRestoreFileLabel(mgr, file);
+    return virSecuritySELinuxRestoreFileLabel(mgr, file, false);
 }
 
 static int
@@ -2063,7 +2140,7 @@ virSecuritySELinuxRestoreUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecuritySELinuxRestoreFileLabel(mgr, file);
+    return virSecuritySELinuxRestoreFileLabel(mgr, file, false);
 }
 
 
@@ -2080,7 +2157,7 @@ virSecuritySELinuxRestoreSCSILabel(virSCSIDevicePtr dev,
     if (virSCSIDeviceGetShareable(dev) || virSCSIDeviceGetReadonly(dev))
         return 0;
 
-    return virSecuritySELinuxRestoreFileLabel(mgr, file);
+    return virSecuritySELinuxRestoreFileLabel(mgr, file, false);
 }
 
 static int
@@ -2090,7 +2167,7 @@ virSecuritySELinuxRestoreHostLabel(virSCSIVHostDevicePtr dev ATTRIBUTE_UNUSED,
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecuritySELinuxRestoreFileLabel(mgr, file);
+    return virSecuritySELinuxRestoreFileLabel(mgr, file, false);
 }
 
 
@@ -2194,7 +2271,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             goto done;
 
-        ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev);
+        ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev, false);
 
         VIR_FREE(vfiodev);
         break;
@@ -2228,7 +2305,7 @@ virSecuritySELinuxRestoreHostdevCapsLabel(virSecurityManagerPtr mgr,
             if (VIR_STRDUP(path, dev->source.caps.u.storage.block) < 0)
                 return -1;
         }
-        ret = virSecuritySELinuxRestoreFileLabel(mgr, path);
+        ret = virSecuritySELinuxRestoreFileLabel(mgr, path, false);
         VIR_FREE(path);
         break;
     }
@@ -2242,7 +2319,7 @@ virSecuritySELinuxRestoreHostdevCapsLabel(virSecurityManagerPtr mgr,
             if (VIR_STRDUP(path, dev->source.caps.u.misc.chardev) < 0)
                 return -1;
         }
-        ret = virSecuritySELinuxRestoreFileLabel(mgr, path);
+        ret = virSecuritySELinuxRestoreFileLabel(mgr, path, false);
         VIR_FREE(path);
         break;
     }
@@ -2390,14 +2467,18 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
     switch (dev_source->type) {
     case VIR_DOMAIN_CHR_TYPE_DEV:
     case VIR_DOMAIN_CHR_TYPE_FILE:
-        if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0)
+        if (virSecuritySELinuxRestoreFileLabel(mgr,
+                                               dev_source->data.file.path,
+                                               false) < 0)
             goto done;
         ret = 0;
         break;
 
     case VIR_DOMAIN_CHR_TYPE_UNIX:
         if (!dev_source->data.nix.listen) {
-            if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0)
+            if (virSecuritySELinuxRestoreFileLabel(mgr,
+                                                   dev_source->data.file.path,
+                                                   false) < 0)
                 goto done;
         }
         ret = 0;
@@ -2408,11 +2489,13 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
             (virAsprintf(&in, "%s.in", dev_source->data.file.path) < 0))
             goto done;
         if (virFileExists(in) && virFileExists(out)) {
-            if ((virSecuritySELinuxRestoreFileLabel(mgr, out) < 0) ||
-                (virSecuritySELinuxRestoreFileLabel(mgr, in) < 0)) {
+            if ((virSecuritySELinuxRestoreFileLabel(mgr, out, false) < 0) ||
+                (virSecuritySELinuxRestoreFileLabel(mgr, in, false) < 0)) {
                 goto done;
             }
-        } else if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) {
+        } else if (virSecuritySELinuxRestoreFileLabel(mgr,
+                                                      dev_source->data.file.path,
+                                                      false) < 0) {
             goto done;
         }
         ret = 0;
@@ -2464,7 +2547,7 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
         database = dev->data.cert.database;
         if (!database)
             database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
-        return virSecuritySELinuxRestoreFileLabel(mgr, database);
+        return virSecuritySELinuxRestoreFileLabel(mgr, database, false);
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
         return virSecuritySELinuxRestoreChardevLabel(mgr, def,
@@ -2559,7 +2642,7 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr,
         rc = -1;
 
     if (def->os.loader && def->os.loader->nvram &&
-        virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram) < 0)
+        virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram, false) < 0)
         rc = -1;
 
     return rc;
@@ -2619,7 +2702,7 @@ virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr,
     if (!secdef || !secdef->relabel)
         return 0;
 
-    return virSecuritySELinuxRestoreFileLabel(mgr, savefile);
+    return virSecuritySELinuxRestoreFileLabel(mgr, savefile, false);
 }
 
 
@@ -3214,7 +3297,7 @@ virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr mgr,
     char *filename = NULL;
     DIR *dir;
 
-    if ((ret = virSecuritySELinuxRestoreFileLabel(mgr, path)))
+    if ((ret = virSecuritySELinuxRestoreFileLabel(mgr, path, false)))
         return ret;
 
     if (!virFileIsDir(path))
@@ -3231,7 +3314,7 @@ virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr mgr,
             ret = -1;
             break;
         }
-        ret = virSecuritySELinuxRestoreFileLabel(mgr, filename);
+        ret = virSecuritySELinuxRestoreFileLabel(mgr, filename, false);
         VIR_FREE(filename);
         if (ret < 0)
             break;
-- 
2.19.2

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

On 12/12/18 7:40 AM, Michal Privoznik wrote:
> Similarly to what I did in DAC driver, this also requires the
> same SELinux label to be used for shared paths. If a path is
> already in use by a domain (or domains) then and the domain we
> are starting now wants to access the path it has to have the same
> SELinux label. This might look too restrictive as the new label
> can still guarantee access to already running domains but in
> reality it is very unlikely and usually an admin mistake.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/security/security_selinux.c | 177 +++++++++++++++++++++++---------
>  1 file changed, 130 insertions(+), 47 deletions(-)
> 

[...]

> +
> +static int
> +virSecuritySELinuxRecallLabel(const char *path,
> +                              security_context_t *con)
> +{
> +    if (virSecurityGetRememberedLabel(SECURITY_SELINUX_NAME,
> +                                      path, con) < 0)
> +        return -1;
> +
> +    if (!con)
> +        return 1;

This ordering of the !con check has caused a Coverity concern that we
use @con in the first call... When compared to the *_dac.c code which
passes &label, I assume this should be passing &con, right?

I'd usually send a patch, but wanted to make sure it was the right option...

John

> +
> +    return 0;
> +}
> +
> +

[...]

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

On 12/20/18 12:39 AM, John Ferlan wrote:
> 
> 
> On 12/12/18 7:40 AM, Michal Privoznik wrote:
>> Similarly to what I did in DAC driver, this also requires the
>> same SELinux label to be used for shared paths. If a path is
>> already in use by a domain (or domains) then and the domain we
>> are starting now wants to access the path it has to have the same
>> SELinux label. This might look too restrictive as the new label
>> can still guarantee access to already running domains but in
>> reality it is very unlikely and usually an admin mistake.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/security/security_selinux.c | 177 +++++++++++++++++++++++---------
>>  1 file changed, 130 insertions(+), 47 deletions(-)
>>
> 
> [...]
> 
>> +
>> +static int
>> +virSecuritySELinuxRecallLabel(const char *path,
>> +                              security_context_t *con)
>> +{
>> +    if (virSecurityGetRememberedLabel(SECURITY_SELINUX_NAME,
>> +                                      path, con) < 0)
>> +        return -1;
>> +
>> +    if (!con)
>> +        return 1;
> 
> This ordering of the !con check has caused a Coverity concern that we
> use @con in the first call... When compared to the *_dac.c code which
> passes &label, I assume this should be passing &con, right?

Ooops, this hould have been if (!*con) return 1;.
security_context_t is actually char *; therefore here con is type of
char ** (just look at virSecurityGetRememberedLabel).

I wonder if this will fix the issue Marc reported (unfortunately I don't
have much time to dig into it right now).

Michal

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

On 12/12/18 7:40 AM, Michal Privoznik wrote:
> Similarly to what I did in DAC driver, this also requires the
> same SELinux label to be used for shared paths. If a path is
> already in use by a domain (or domains) then and the domain we
> are starting now wants to access the path it has to have the same
> SELinux label. This might look too restrictive as the new label
> can still guarantee access to already running domains but in
> reality it is very unlikely and usually an admin mistake.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/security/security_selinux.c | 177 +++++++++++++++++++++++---------
>  1 file changed, 130 insertions(+), 47 deletions(-)
> 

[...]

>  static int
> @@ -1362,7 +1429,8 @@ getContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>   * errors that the caller(s) are already dealing with */
>  static int
>  virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
> -                                   const char *path)
> +                                   const char *path,
> +                                   bool recall)
>  {
>      bool privileged = virSecurityManagerGetPrivileged(mgr);
>      struct stat buf;
> @@ -1386,26 +1454,35 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
>          goto cleanup;
>      }
>  
> -    if (stat(newpath, &buf) != 0) {
> -        VIR_WARN("cannot stat %s: %s", newpath,
> -                 virStrerror(errno, ebuf, sizeof(ebuf)));
> -        goto cleanup;
> -    }
> -
> -    if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
> -        /* Any user created path likely does not have a default label,
> -         * which makes this an expected non error
> -         */
> -        VIR_WARN("cannot lookup default selinux label for %s", newpath);
> -        ret = 0;
> -        goto cleanup;
> -    }
> -
> -    if ((rc = virSecuritySELinuxTransactionAppend(path, fcon, false, true)) < 0)
> +    if ((rc = virSecuritySELinuxTransactionAppend(path, NULL, false, true)) < 0)
>          return -1;
>      else if (rc > 0)
>          return 0;
>  

Since you've touched the code, Coverity looks again and determines that
@newpath can be leaked above

John

> +    if (recall) {
> +        if ((rc = virSecuritySELinuxRecallLabel(newpath, &fcon)) < 0) {
> +            goto cleanup;
> +        } else if (rc > 0) {
> +            ret = 0;
> +            goto cleanup;
> +        }
> +    } else {
> +        if (stat(newpath, &buf) != 0) {
> +            VIR_WARN("cannot stat %s: %s", newpath,
> +                     virStrerror(errno, ebuf, sizeof(ebuf)));
> +            goto cleanup;
> +        }
> +
> +        if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
> +            /* Any user created path likely does not have a default label,
> +             * which makes this an expected non error
> +             */
> +            VIR_WARN("cannot lookup default selinux label for %s", newpath);
> +            ret = 0;
> +            goto cleanup;
> +        }
> +    }
> +

[...]

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

On 12/20/18 12:39 AM, John Ferlan wrote:
> 
> 
> On 12/12/18 7:40 AM, Michal Privoznik wrote:
>> Similarly to what I did in DAC driver, this also requires the
>> same SELinux label to be used for shared paths. If a path is
>> already in use by a domain (or domains) then and the domain we
>> are starting now wants to access the path it has to have the same
>> SELinux label. This might look too restrictive as the new label
>> can still guarantee access to already running domains but in
>> reality it is very unlikely and usually an admin mistake.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/security/security_selinux.c | 177 +++++++++++++++++++++++---------
>>  1 file changed, 130 insertions(+), 47 deletions(-)
>>
> 
> [...]
> 
>>  static int
>> @@ -1362,7 +1429,8 @@ getContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>>   * errors that the caller(s) are already dealing with */
>>  static int
>>  virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
>> -                                   const char *path)
>> +                                   const char *path,
>> +                                   bool recall)
>>  {
>>      bool privileged = virSecurityManagerGetPrivileged(mgr);
>>      struct stat buf;
>> @@ -1386,26 +1454,35 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
>>          goto cleanup;
>>      }
>>  
>> -    if (stat(newpath, &buf) != 0) {
>> -        VIR_WARN("cannot stat %s: %s", newpath,
>> -                 virStrerror(errno, ebuf, sizeof(ebuf)));
>> -        goto cleanup;
>> -    }
>> -
>> -    if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
>> -        /* Any user created path likely does not have a default label,
>> -         * which makes this an expected non error
>> -         */
>> -        VIR_WARN("cannot lookup default selinux label for %s", newpath);
>> -        ret = 0;
>> -        goto cleanup;
>> -    }
>> -
>> -    if ((rc = virSecuritySELinuxTransactionAppend(path, fcon, false, true)) < 0)
>> +    if ((rc = virSecuritySELinuxTransactionAppend(path, NULL, false, true)) < 0)
>>          return -1;
>>      else if (rc > 0)
>>          return 0;
>>  
> 
> Since you've touched the code, Coverity looks again and determines that
> @newpath can be leaked above

Ah, right. this should have been "goto cleanup" instead of "return -1"
and "{ret = 0; goto cleanup}" instead of "return 0".

Michal

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