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>

Split Close
Expand all
Collapse all
          --- old/usr/src/lib/libzfs/common/libzfs_import.c
          +++ new/usr/src/lib/libzfs/common/libzfs_import.c
↓ open down ↓ 232 lines elided ↑ open up ↑
 233  233                  if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
 234  234                          return (-1);
 235  235  
 236  236                  if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
 237  237                          free(ne);
 238  238                          return (-1);
 239  239                  }
 240  240                  ne->ne_guid = vdev_guid;
 241  241                  ne->ne_next = pl->names;
 242  242                  pl->names = ne;
      243 +                nvlist_free(config);
 243  244                  return (0);
 244  245          }
 245  246  
 246  247          /*
 247  248           * If we have a valid config but cannot read any of these fields, then
 248  249           * it means we have a half-initialized label.  In vdev_label_init()
 249  250           * we write a label with txg == 0 so that we can identify the device
 250  251           * in case the user refers to the same disk later on.  If we fail to
 251  252           * create the pool, we'll be left with a label in this state
 252  253           * which should not be considered part of a valid pool.
↓ open down ↓ 13 lines elided ↑ open up ↑
 266  267          /*
 267  268           * First, see if we know about this pool.  If not, then add it to the
 268  269           * list of known pools.
 269  270           */
 270  271          for (pe = pl->pools; pe != NULL; pe = pe->pe_next) {
 271  272                  if (pe->pe_guid == pool_guid)
 272  273                          break;
 273  274          }
 274  275  
 275  276          if (pe == NULL) {
 276      -                if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL) {
 277      -                        nvlist_free(config);
      277 +                if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL)
 278  278                          return (-1);
 279      -                }
 280  279                  pe->pe_guid = pool_guid;
 281  280                  pe->pe_next = pl->pools;
 282  281                  pl->pools = pe;
 283  282          }
 284  283  
 285  284          /*
 286  285           * Second, see if we know about this toplevel vdev.  Add it if its
 287  286           * missing.
 288  287           */
 289  288          for (ve = pe->pe_vdevs; ve != NULL; ve = ve->ve_next) {
 290  289                  if (ve->ve_guid == top_guid)
 291  290                          break;
 292  291          }
 293  292  
 294  293          if (ve == NULL) {
 295      -                if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL) {
 296      -                        nvlist_free(config);
      294 +                if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL)
 297  295                          return (-1);
 298      -                }
 299  296                  ve->ve_guid = top_guid;
 300  297                  ve->ve_next = pe->pe_vdevs;
 301  298                  pe->pe_vdevs = ve;
 302  299          }
 303  300  
 304  301          /*
 305      -         * Third, see if we have a config with a matching transaction group.  If
 306      -         * so, then we do nothing.  Otherwise, add it to the list of known
 307      -         * configs.
      302 +         * Third, add the vdev guid -> path mappings so that we can fix up
      303 +         * the configuration as necessary before doing the import.
      304 +         */
      305 +        if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
      306 +                return (-1);
      307 +
      308 +        if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
      309 +                free(ne);
      310 +                return (-1);
      311 +        }
      312 +
      313 +        ne->ne_guid = vdev_guid;
      314 +        ne->ne_next = pl->names;
      315 +        pl->names = ne;
      316 +
      317 +        /*
      318 +         * Finally, see if we have a config with a matching transaction
      319 +         * group.  If so, then we do nothing.  Otherwise, add it to the list
      320 +         * of known configs.
 308  321           */
 309  322          for (ce = ve->ve_configs; ce != NULL; ce = ce->ce_next) {
 310  323                  if (ce->ce_txg == txg)
 311  324                          break;
 312  325          }
 313  326  
 314  327          if (ce == NULL) {
 315      -                if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL) {
 316      -                        nvlist_free(config);
      328 +                if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL)
 317  329                          return (-1);
 318      -                }
 319  330                  ce->ce_txg = txg;
 320  331                  ce->ce_config = config;
 321  332                  ce->ce_next = ve->ve_configs;
 322  333                  ve->ve_configs = ce;
 323  334          } else {
 324  335                  nvlist_free(config);
 325  336          }
 326  337  
 327      -        /*
 328      -         * At this point we've successfully added our config to the list of
 329      -         * known configs.  The last thing to do is add the vdev guid -> path
 330      -         * mappings so that we can fix up the configuration as necessary before
 331      -         * doing the import.
 332      -         */
 333      -        if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
 334      -                return (-1);
 335      -
 336      -        if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
 337      -                free(ne);
 338      -                return (-1);
 339      -        }
 340      -
 341      -        ne->ne_guid = vdev_guid;
 342      -        ne->ne_next = pl->names;
 343      -        pl->names = ne;
 344      -
 345  338          return (0);
 346  339  }
 347  340  
 348  341  /*
 349  342   * Returns true if the named pool matches the given GUID.
 350  343   */
 351  344  static int
 352  345  pool_active(libzfs_handle_t *hdl, const char *name, uint64_t guid,
 353  346      boolean_t *isactive)
 354  347  {
↓ open down ↓ 896 lines elided ↑ open up ↑
1251 1244                                  }
1252 1245                                  if (!matched) {
1253 1246                                          nvlist_free(config);
1254 1247                                  } else {
1255 1248                                          /*
1256 1249                                           * use the non-raw path for the config
1257 1250                                           */
1258 1251                                          (void) strlcpy(end, slice->rn_name,
1259 1252                                              pathleft);
1260 1253                                          if (add_config(hdl, &pools, path,
1261      -                                            config) != 0)
     1254 +                                            config) != 0) {
     1255 +                                                nvlist_free(config);
1262 1256                                                  config_failed = B_TRUE;
     1257 +                                        }
1263 1258                                  }
1264 1259                          }
1265 1260                          free(slice->rn_name);
1266 1261                          free(slice);
1267 1262                  }
1268 1263                  avl_destroy(&slice_cache);
1269 1264  
1270 1265                  (void) closedir(dirp);
1271 1266  
1272 1267                  if (config_failed)
↓ open down ↓ 428 lines elided ↑ open up ↑
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX