| Message ID | 1397567405-5863-3-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Vinod Koul |
| Headers | show |
On Tue, 2014-04-15 at 16:10 +0300, Andy Shevchenko wrote: > When we fail to allocate memory for thread->srcs or thread->dsts and src_cnt or > dst_cnt great than 1 we leak memory on error path. This patch fixes the issue. > As a side effect we allocate the exact number of soruce and destination > buffers. Dan, ping? > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/dma/dmatest.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > index 0593baa..94b1694 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -440,7 +440,7 @@ static int dmatest_func(void *data) > src_cnt = min_odd(params->pq_sources | 1, dma_maxpq(dev, 0)); > dst_cnt = 2; > > - pq_coefs = kmalloc(params->pq_sources+1, GFP_KERNEL); > + pq_coefs = kmalloc(params->pq_sources + 1, GFP_KERNEL); > if (!pq_coefs) > goto err_thread_type; > > @@ -449,7 +449,7 @@ static int dmatest_func(void *data) > } else > goto err_thread_type; > > - thread->srcs = kcalloc(src_cnt+1, sizeof(u8 *), GFP_KERNEL); > + thread->srcs = kcalloc(src_cnt, sizeof(u8 *), GFP_KERNEL); > if (!thread->srcs) > goto err_srcs; > for (i = 0; i < src_cnt; i++) { > @@ -457,9 +457,8 @@ static int dmatest_func(void *data) > if (!thread->srcs[i]) > goto err_srcbuf; > } > - thread->srcs[i] = NULL; > > - thread->dsts = kcalloc(dst_cnt+1, sizeof(u8 *), GFP_KERNEL); > + thread->dsts = kcalloc(dst_cnt, sizeof(u8 *), GFP_KERNEL); > if (!thread->dsts) > goto err_dsts; > for (i = 0; i < dst_cnt; i++) { > @@ -467,7 +466,6 @@ static int dmatest_func(void *data) > if (!thread->dsts[i]) > goto err_dstbuf; > } > - thread->dsts[i] = NULL; > > set_user_nice(current, 10); > > @@ -749,14 +747,17 @@ static int dmatest_func(void *data) > runtime = ktime_us_delta(ktime_get(), ktime); > > ret = 0; > - for (i = 0; thread->dsts[i]; i++) > - kfree(thread->dsts[i]); > + > + i = dst_cnt; > err_dstbuf: > + while (--i >= 0) > + kfree(thread->dsts[i]); > kfree(thread->dsts); > err_dsts: > - for (i = 0; thread->srcs[i]; i++) > - kfree(thread->srcs[i]); > + i = src_cnt; > err_srcbuf: > + while (--i >= 0) > + kfree(thread->srcs[i]); > kfree(thread->srcs); > err_srcs: > kfree(pq_coefs);
On Tue, Apr 29, 2014 at 1:17 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Tue, 2014-04-15 at 16:10 +0300, Andy Shevchenko wrote: >> When we fail to allocate memory for thread->srcs or thread->dsts and src_cnt or >> dst_cnt great than 1 we leak memory on error path. This patch fixes the issue. >> As a side effect we allocate the exact number of soruce and destination >> buffers. > > Dan, ping? > Hi Andy, Can you drop the cleanups out of this fix patch. I'm inclined to leave them as long as checkpatch does not wine about them, otherwise a separate cleanup patch is preferred. -- Dan -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c index 0593baa..94b1694 100644 --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -440,7 +440,7 @@ static int dmatest_func(void *data) src_cnt = min_odd(params->pq_sources | 1, dma_maxpq(dev, 0)); dst_cnt = 2; - pq_coefs = kmalloc(params->pq_sources+1, GFP_KERNEL); + pq_coefs = kmalloc(params->pq_sources + 1, GFP_KERNEL); if (!pq_coefs) goto err_thread_type; @@ -449,7 +449,7 @@ static int dmatest_func(void *data) } else goto err_thread_type; - thread->srcs = kcalloc(src_cnt+1, sizeof(u8 *), GFP_KERNEL); + thread->srcs = kcalloc(src_cnt, sizeof(u8 *), GFP_KERNEL); if (!thread->srcs) goto err_srcs; for (i = 0; i < src_cnt; i++) { @@ -457,9 +457,8 @@ static int dmatest_func(void *data) if (!thread->srcs[i]) goto err_srcbuf; } - thread->srcs[i] = NULL; - thread->dsts = kcalloc(dst_cnt+1, sizeof(u8 *), GFP_KERNEL); + thread->dsts = kcalloc(dst_cnt, sizeof(u8 *), GFP_KERNEL); if (!thread->dsts) goto err_dsts; for (i = 0; i < dst_cnt; i++) { @@ -467,7 +466,6 @@ static int dmatest_func(void *data) if (!thread->dsts[i]) goto err_dstbuf; } - thread->dsts[i] = NULL; set_user_nice(current, 10); @@ -749,14 +747,17 @@ static int dmatest_func(void *data) runtime = ktime_us_delta(ktime_get(), ktime); ret = 0; - for (i = 0; thread->dsts[i]; i++) - kfree(thread->dsts[i]); + + i = dst_cnt; err_dstbuf: + while (--i >= 0) + kfree(thread->dsts[i]); kfree(thread->dsts); err_dsts: - for (i = 0; thread->srcs[i]; i++) - kfree(thread->srcs[i]); + i = src_cnt; err_srcbuf: + while (--i >= 0) + kfree(thread->srcs[i]); kfree(thread->srcs); err_srcs: kfree(pq_coefs);
When we fail to allocate memory for thread->srcs or thread->dsts and src_cnt or dst_cnt great than 1 we leak memory on error path. This patch fixes the issue. As a side effect we allocate the exact number of soruce and destination buffers. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/dma/dmatest.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)