Commit e9777ad4 authored by Sebastian Sanchez's avatar Sebastian Sanchez Committed by Doug Ledford

IB/{hfi1, rdmavt}: Fix memory leak in hfi1_alloc_devdata() upon failure

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: default avatarMike Marciniszyn <mike.marciniszyn@intel.com>
Reviewed-by: default avatarMichael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: default avatarSebastian Sanchez <sebastian.sanchez@intel.com>
Signed-off-by: default avatarDennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: default avatarDoug Ledford <dledford@redhat.com>
parent 45d92457
...@@ -1209,30 +1209,49 @@ static void finalize_asic_data(struct hfi1_devdata *dd, ...@@ -1209,30 +1209,49 @@ static void finalize_asic_data(struct hfi1_devdata *dd,
kfree(ad); 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; struct hfi1_asic_data *ad;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&hfi1_devs_lock, flags); spin_lock_irqsave(&hfi1_devs_lock, flags);
idr_remove(&hfi1_unit_table, dd->unit); if (!list_empty(&dd->list)) {
list_del(&dd->list); idr_remove(&hfi1_unit_table, dd->unit);
list_del_init(&dd->list);
}
ad = release_asic_data(dd); ad = release_asic_data(dd);
spin_unlock_irqrestore(&hfi1_devs_lock, flags); spin_unlock_irqrestore(&hfi1_devs_lock, flags);
if (ad)
finalize_asic_data(dd, ad); finalize_asic_data(dd, ad);
free_platform_config(dd); free_platform_config(dd);
rcu_barrier(); /* wait for rcu callbacks to complete */ rcu_barrier(); /* wait for rcu callbacks to complete */
free_percpu(dd->int_counter); free_percpu(dd->int_counter);
free_percpu(dd->rcv_limit); free_percpu(dd->rcv_limit);
free_percpu(dd->send_schedule); free_percpu(dd->send_schedule);
free_percpu(dd->tx_opstats); 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); sdma_clean(dd, dd->num_sdma);
rvt_dealloc_device(&dd->verbs_dev.rdi); 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 = { static struct kobj_type hfi1_devdata_type = {
.release = __hfi1_free_devdata, .release = __hfi1_free_devdata,
}; };
...@@ -1333,9 +1352,7 @@ struct hfi1_devdata *hfi1_alloc_devdata(struct pci_dev *pdev, size_t extra) ...@@ -1333,9 +1352,7 @@ struct hfi1_devdata *hfi1_alloc_devdata(struct pci_dev *pdev, size_t extra)
return dd; return dd;
bail: bail:
if (!list_empty(&dd->list)) hfi1_clean_devdata(dd);
list_del_init(&dd->list);
rvt_dealloc_device(&dd->verbs_dev.rdi);
return ERR_PTR(ret); return ERR_PTR(ret);
} }
......
...@@ -199,6 +199,7 @@ void free_platform_config(struct hfi1_devdata *dd) ...@@ -199,6 +199,7 @@ void free_platform_config(struct hfi1_devdata *dd)
{ {
/* Release memory allocated for eprom or fallback file read. */ /* Release memory allocated for eprom or fallback file read. */
kfree(dd->platform_config.data); kfree(dd->platform_config.data);
dd->platform_config.data = NULL;
} }
void get_port_type(struct hfi1_pportdata *ppd) void get_port_type(struct hfi1_pportdata *ppd)
......
...@@ -204,6 +204,8 @@ static void clean_i2c_bus(struct hfi1_i2c_bus *bus) ...@@ -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) void clean_up_i2c(struct hfi1_devdata *dd, struct hfi1_asic_data *ad)
{ {
if (!ad)
return;
clean_i2c_bus(ad->i2c_bus0); clean_i2c_bus(ad->i2c_bus0);
ad->i2c_bus0 = NULL; ad->i2c_bus0 = NULL;
clean_i2c_bus(ad->i2c_bus1); clean_i2c_bus(ad->i2c_bus1);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment