[libvirt] [PATCH] storage: Avoid memory leak on metadata fetching

[Michal Privoznik]

Getting metadata on storage allocates a memory (path) which need to
be freed after use otherwise it gets leaked. This means after use of
virStorageFileGetMetadataFromFD or virStorageFileGetMetadata one
must call virStorageFileFreeMetadata to free it. Right now this
function frees only one variable in metadata structure, but is prepared
to be extended in the future.
---
 src/conf/domain_conf.c           |    8 +++++++-
 src/libvirt_private.syms         |    1 +
 src/qemu/qemu_driver.c           |    3 +++
 src/storage/storage_backend_fs.c |    5 +++--
 src/util/storage_file.c          |   15 +++++++++++++++
 src/util/storage_file.h          |    2 ++
 6 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 39e59b9..2ebd012 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11055,6 +11055,9 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
     int ret = -1;
     size_t depth = 0;
     char *nextpath = NULL;
+    virStorageFileMetadata meta;
+
+    memset(&meta, 0, sizeof(virStorageFileMetadata));
 
     if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
         return 0;
@@ -11084,7 +11087,6 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
     paths = virHashCreate(5, NULL);
 
     do {
-        virStorageFileMetadata meta;
         const char *path = nextpath ? nextpath : disk->src;
         int fd;
 
@@ -11127,6 +11129,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
 
         depth++;
         nextpath = meta.backingStore;
+        meta.backingStore = NULL;
 
         /* Stop iterating if we reach a non-file backing store */
         if (nextpath && !meta.backingStoreIsFile) {
@@ -11137,6 +11140,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
 
         format = meta.backingStoreFormat;
 
+        virStorageFileFreeMetadata(meta);
+
         if (format == VIR_STORAGE_FILE_AUTO &&
             !allowProbing)
             format = VIR_STORAGE_FILE_RAW; /* Stops further recursion */
@@ -11151,6 +11156,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
 cleanup:
     virHashFree(paths);
     VIR_FREE(nextpath);
+    virStorageFileFreeMetadata(meta);
 
     return ret;
 }
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3237d18..e06c7fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -940,6 +940,7 @@ virStorageGenerateQcowPassphrase;
 # storage_file.h
 virStorageFileFormatTypeFromString;
 virStorageFileFormatTypeToString;
+virStorageFileFreeMetadata;
 virStorageFileGetMetadata;
 virStorageFileGetMetadataFromFD;
 virStorageFileIsSharedFS;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0a6b48e..54f9bba 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6358,6 +6358,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
 
     virCheckFlags(0, -1);
 
+    memset(&meta, 0, sizeof(virStorageFileMetadata));
+
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     qemuDriverUnlock(driver);
@@ -6511,6 +6513,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
 
 cleanup:
     VIR_FORCE_CLOSE(fd);
+    virStorageFileFreeMetadata(meta);
     if (vm)
         virDomainObjUnlock(vm);
     return ret;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index d87401f..02b1637 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -118,7 +118,6 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
             ret = 0;
         }
     } else {
-        VIR_FREE(meta.backingStore);
         ret = 0;
     }
 
@@ -147,10 +146,12 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
          */
     }
 
+    virStorageFileFreeMetadata(meta);
+
     return ret;
 
 cleanup:
-    VIR_FREE(*backingStore);
+    virStorageFileFreeMetadata(meta);
     return -1;
 }
 
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 06cabc8..19c2464 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -819,6 +819,8 @@ virStorageFileProbeFormat(const char *path)
  * it indicates the image didn't specify an explicit format for its
  * backing store. Callers are advised against probing for the
  * backing store format in this case.
+ *
+ * Caller must free @meta after use via virStorageFileFreeMetadata.
  */
 int
 virStorageFileGetMetadataFromFD(const char *path,
@@ -892,6 +894,8 @@ cleanup:
  * it indicates the image didn't specify an explicit format for its
  * backing store. Callers are advised against probing for the
  * backing store format in this case.
+ *
+ * Caller MUST free @meta after use via virStorageFileFreeMetadata.
  */
 int
 virStorageFileGetMetadata(const char *path,
@@ -912,6 +916,17 @@ virStorageFileGetMetadata(const char *path,
     return ret;
 }
 
+/**
+ * virStorageFileFreeMetadata:
+ *
+ * Free pointers in passed structure, but not memory used by
+ * structure itself.
+ */
+void
+virStorageFileFreeMetadata(virStorageFileMetadata meta)
+{
+    VIR_FREE(meta.backingStore);
+}
 
 #ifdef __linux__
 
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index f1bdd02..b362796 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -70,6 +70,8 @@ int virStorageFileGetMetadataFromFD(const char *path,
                                     int format,
                                     virStorageFileMetadata *meta);
 
+void virStorageFileFreeMetadata(virStorageFileMetadata meta);
+
 enum {
     VIR_STORAGE_FILE_SHFS_NFS = (1 << 0),
     VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1),
-- 
1.7.5.rc3