Print this page
patch v2
6120 libzfs leaks a config nvlist for spares and l2arc
Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>

@@ -238,10 +238,11 @@
                         return (-1);
                 }
                 ne->ne_guid = vdev_guid;
                 ne->ne_next = pl->names;
                 pl->names = ne;
+                nvlist_free(config);
                 return (0);
         }
 
         /*
          * If we have a valid config but cannot read any of these fields, then

@@ -271,14 +272,12 @@
                 if (pe->pe_guid == pool_guid)
                         break;
         }
 
         if (pe == NULL) {
-                if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL) {
-                        nvlist_free(config);
+                if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL)
                         return (-1);
-                }
                 pe->pe_guid = pool_guid;
                 pe->pe_next = pl->pools;
                 pl->pools = pe;
         }
 

@@ -290,60 +289,54 @@
                 if (ve->ve_guid == top_guid)
                         break;
         }
 
         if (ve == NULL) {
-                if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL) {
-                        nvlist_free(config);
+                if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL)
                         return (-1);
-                }
                 ve->ve_guid = top_guid;
                 ve->ve_next = pe->pe_vdevs;
                 pe->pe_vdevs = ve;
         }
 
         /*
-         * Third, see if we have a config with a matching transaction group.  If
-         * so, then we do nothing.  Otherwise, add it to the list of known
-         * configs.
+         * Third, add the vdev guid -> path mappings so that we can fix up
+         * the configuration as necessary before doing the import.
+         */
+        if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
+                return (-1);
+
+        if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
+                free(ne);
+                return (-1);
+        }
+
+        ne->ne_guid = vdev_guid;
+        ne->ne_next = pl->names;
+        pl->names = ne;
+
+        /*
+         * Finally, see if we have a config with a matching transaction
+         * group.  If so, then we do nothing.  Otherwise, add it to the list
+         * of known configs.
          */
         for (ce = ve->ve_configs; ce != NULL; ce = ce->ce_next) {
                 if (ce->ce_txg == txg)
                         break;
         }
 
         if (ce == NULL) {
-                if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL) {
-                        nvlist_free(config);
+                if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL)
                         return (-1);
-                }
                 ce->ce_txg = txg;
                 ce->ce_config = config;
                 ce->ce_next = ve->ve_configs;
                 ve->ve_configs = ce;
         } else {
                 nvlist_free(config);
         }
 
-        /*
-         * At this point we've successfully added our config to the list of
-         * known configs.  The last thing to do is add the vdev guid -> path
-         * mappings so that we can fix up the configuration as necessary before
-         * doing the import.
-         */
-        if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
-                return (-1);
-
-        if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
-                free(ne);
-                return (-1);
-        }
-
-        ne->ne_guid = vdev_guid;
-        ne->ne_next = pl->names;
-        pl->names = ne;
-
         return (0);
 }
 
 /*
  * Returns true if the named pool matches the given GUID.

@@ -1256,14 +1249,16 @@
                                          * use the non-raw path for the config
                                          */
                                         (void) strlcpy(end, slice->rn_name,
                                             pathleft);
                                         if (add_config(hdl, &pools, path,
-                                            config) != 0)
+                                            config) != 0) {
+                                                nvlist_free(config);
                                                 config_failed = B_TRUE;
                                 }
                         }
+                        }
                         free(slice->rn_name);
                         free(slice);
                 }
                 avl_destroy(&slice_cache);