| Message ID | 20200409174113.28635-1-keitasuzuki.park@sslab.ics.keio.ac.jp |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | nfp: Fix memory leak in nfp_resource_acquire() | expand |
On Thu, 9 Apr 2020 17:41:11 +0000 Keita Suzuki wrote: > This patch fixes a memory leak in nfp_resource_acquire(). res->mutex is > alllocated in nfp_resource_try_acquire(). However, when > msleep_interruptible() or time_is_before_eq_jiffies() fails, it falls > into err_fails path where res is freed, but res->mutex is not. > > Fix this by changing call to free to nfp_resource_release(). I don't see a leak here. Maybe you could rephrase the description to make things clearer? AFAICS nfp_resource_try_acquire() calls nfp_cpp_mutex_free(res->mutex) if it's about to return an error. We can only hit msleep or time check if it returned an error.
Hi, Thanks for reviewing. I'll check back the runtime log and see what I can do. Thanks. 2020年4月10日(金) 4:32 Jakub Kicinski <kuba@kernel.org>: > > On Thu, 9 Apr 2020 17:41:11 +0000 Keita Suzuki wrote: > > This patch fixes a memory leak in nfp_resource_acquire(). res->mutex is > > alllocated in nfp_resource_try_acquire(). However, when > > msleep_interruptible() or time_is_before_eq_jiffies() fails, it falls > > into err_fails path where res is freed, but res->mutex is not. > > > > Fix this by changing call to free to nfp_resource_release(). > > I don't see a leak here. Maybe you could rephrase the description to > make things clearer? > > AFAICS nfp_resource_try_acquire() calls nfp_cpp_mutex_free(res->mutex) > if it's about to return an error. We can only hit msleep or time check > if it returned an error.
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c index ce7492a6a98f..95e7bdc95652 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c @@ -200,7 +200,7 @@ nfp_resource_acquire(struct nfp_cpp *cpp, const char *name) err_free: nfp_cpp_mutex_free(dev_mutex); - kfree(res); + nfp_resource_release(res); return ERR_PTR(err); }
This patch fixes a memory leak in nfp_resource_acquire(). res->mutex is alllocated in nfp_resource_try_acquire(). However, when msleep_interruptible() or time_is_before_eq_jiffies() fails, it falls into err_fails path where res is freed, but res->mutex is not. Fix this by changing call to free to nfp_resource_release(). Signed-off-by: Keita Suzuki <keitasuzuki.park@sslab.ics.keio.ac.jp> --- drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)