IB/{hfi1, rdmavt}: Fix memory leak in hfi1_alloc_devdata() upon failure
authorSebastian Sanchez <sebastian.sanchez@intel.com>
Tue, 1 May 2018 12:36:06 +0000 (05:36 -0700)
committerDoug Ledford <dledford@redhat.com>
Thu, 3 May 2018 19:24:48 +0000 (15:24 -0400)
When allocating device data, if there's an allocation failure, the
already allocated memory won't be freed such as per-cpu counters.

Fix memory leaks in exception path by creating a common reentrant
clean up function hfi1_clean_devdata() to be used at driver unload
time and device data allocation failure.

To accomplish this, free_platform_config() and clean_up_i2c() are
changed to be reentrant to remove dependencies when they are called
in different order. This helps avoid NULL pointer dereferences
introduced by this patch if those two functions weren't reentrant.

In addition, set dd->int_counter, dd->rcv_limit,
dd->send_schedule and dd->tx_opstats to NULL after they're freed in
hfi1_clean_devdata(), so that hfi1_clean_devdata() is fully reentrant.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Sebastian Sanchez <sebastian.sanchez@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
drivers/infiniband/hw/hfi1/init.c
drivers/infiniband/hw/hfi1/platform.c
drivers/infiniband/hw/hfi1/qsfp.c

index b417e3b40e4a84cdcf71afeb41c0e5713ebd63d7..6309edf811df6e33d2ea3f64e6d7592231c8dd3a 100644 (file)
@@ -1209,30 +1209,49 @@ static void finalize_asic_data(struct hfi1_devdata *dd,
        kfree(ad);
 }
 
-static void __hfi1_free_devdata(struct kobject *kobj)
+/**
+ * hfi1_clean_devdata - cleans up per-unit data structure
+ * @dd: pointer to a valid devdata structure
+ *
+ * It cleans up all data structures set up by
+ * by hfi1_alloc_devdata().
+ */
+static void hfi1_clean_devdata(struct hfi1_devdata *dd)
 {
-       struct hfi1_devdata *dd =
-               container_of(kobj, struct hfi1_devdata, kobj);
        struct hfi1_asic_data *ad;
        unsigned long flags;
 
        spin_lock_irqsave(&hfi1_devs_lock, flags);
-       idr_remove(&hfi1_unit_table, dd->unit);
-       list_del(&dd->list);
+       if (!list_empty(&dd->list)) {
+               idr_remove(&hfi1_unit_table, dd->unit);
+               list_del_init(&dd->list);
+       }
        ad = release_asic_data(dd);
        spin_unlock_irqrestore(&hfi1_devs_lock, flags);
-       if (ad)
-               finalize_asic_data(dd, ad);
+
+       finalize_asic_data(dd, ad);
        free_platform_config(dd);
        rcu_barrier(); /* wait for rcu callbacks to complete */
        free_percpu(dd->int_counter);
        free_percpu(dd->rcv_limit);
        free_percpu(dd->send_schedule);
        free_percpu(dd->tx_opstats);
+       dd->int_counter   = NULL;
+       dd->rcv_limit     = NULL;
+       dd->send_schedule = NULL;
+       dd->tx_opstats    = NULL;
        sdma_clean(dd, dd->num_sdma);
        rvt_dealloc_device(&dd->verbs_dev.rdi);
 }
 
+static void __hfi1_free_devdata(struct kobject *kobj)
+{
+       struct hfi1_devdata *dd =
+               container_of(kobj, struct hfi1_devdata, kobj);
+
+       hfi1_clean_devdata(dd);
+}
+
 static struct kobj_type hfi1_devdata_type = {
        .release = __hfi1_free_devdata,
 };
@@ -1333,9 +1352,7 @@ struct hfi1_devdata *hfi1_alloc_devdata(struct pci_dev *pdev, size_t extra)
        return dd;
 
 bail:
-       if (!list_empty(&dd->list))
-               list_del_init(&dd->list);
-       rvt_dealloc_device(&dd->verbs_dev.rdi);
+       hfi1_clean_devdata(dd);
        return ERR_PTR(ret);
 }
 
index d486355880cb0da37e23755e8ed49197fed90fc8..cbf7faa5038ca9e7e271363ce80f9174b97da907 100644 (file)
@@ -199,6 +199,7 @@ void free_platform_config(struct hfi1_devdata *dd)
 {
        /* Release memory allocated for eprom or fallback file read. */
        kfree(dd->platform_config.data);
+       dd->platform_config.data = NULL;
 }
 
 void get_port_type(struct hfi1_pportdata *ppd)
index 1869f639c3aec7283a2e502b126d5f31573386bb..b5966991d64744e710dbb7e38574b91c676bacf8 100644 (file)
@@ -204,6 +204,8 @@ static void clean_i2c_bus(struct hfi1_i2c_bus *bus)
 
 void clean_up_i2c(struct hfi1_devdata *dd, struct hfi1_asic_data *ad)
 {
+       if (!ad)
+               return;
        clean_i2c_bus(ad->i2c_bus0);
        ad->i2c_bus0 = NULL;
        clean_i2c_bus(ad->i2c_bus1);