security, storage: plug memory leaks for security_context_t

security_context_t happens to be a typedef for char*, and happens to
begin with a string usable as a raw context string.  But in reality,
it is an opaque type that may or may not have additional information
after the first NUL byte, where that additional information can
include pointers that can only be freed via freecon().

Proof is from this valgrind run of daemon/libvirtd:

==6028== 839,169 (40 direct, 839,129 indirect) bytes in 1 blocks are definitely lost in loss record 274 of 274
==6028==    at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==6028==    by 0x3022E0D48C: selabel_open (label.c:165)
==6028==    by 0x3022E11646: matchpathcon_init_prefix (matchpathcon.c:296)
==6028==    by 0x3022E1190D: matchpathcon (matchpathcon.c:317)
==6028==    by 0x4F9D842: SELinuxRestoreSecurityFileLabel (security_selinux.c:382)

800k is a lot of memory to be leaking.

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Avoid leak on error.
* src/security/security_selinux.c
(SELinuxReserveSecurityLabel, SELinuxGetSecurityProcessLabel)
(SELinuxRestoreSecurityFileLabel): Use correct function to free
security_context_t.

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 996177ac059468dcbab919e8a9e13ff0ebb4db0a..2a4517207115d41df14abaec0aeee94f2fc55900 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -239,7 +239,7 @@ SELinuxReserveSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
     }
 
     ctx = context_new(pctx);
-    VIR_FREE(pctx);
+    freecon(pctx);
     if (!ctx)
         goto err;
 
@@ -298,11 +298,12 @@ SELinuxGetSecurityProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
                                _("security label exceeds "
                                  "maximum length: %d"),
                                VIR_SECURITY_LABEL_BUFLEN - 1);
+        freecon(ctx);
         return -1;
     }
 
     strcpy(sec->label, (char *) ctx);
-    VIR_FREE(ctx);
+    freecon(ctx);
 
     sec->enforcing = security_getenforce();
     if (sec->enforcing == -1) {
@@ -387,7 +388,7 @@ SELinuxRestoreSecurityFileLabel(const char *path)
     }
 
 err:
-    VIR_FREE(fcon);
+    freecon(fcon);
     VIR_FREE(newpath);
     return rc;
 }

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a6ee5645139a4aeb0f41e64adf43935eb847a362..10ea33c13380fb3f49233a5cc57af3d6bc46d9e2 100644 (file)
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1148,11 +1148,11 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,
         }
     } else {
         target->perms.label = strdup(filecon);
+        freecon(filecon);
         if (target->perms.label == NULL) {
             virReportOOMError();
             return -1;
         }
-        freecon(filecon);
     }
 #else
     target->perms.label = NULL;