| Message ID | 20200928175945.GA11320@embeddedor (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [next] crypto: sun8i-ss - Fix memory leak in sun8i_ss_prng_generate() | expand |
On Mon, Sep 28, 2020 at 12:59:45PM -0500, Gustavo A. R. Silva wrote: > Set _err_ to the return error code -EFAULT before jumping to the new > label err_d, so resources for _d_ can be released before returning > from function sun8i_ss_prng_generate(). > > Addresses-Coverity-ID: 1497459 ("Resource leak") > Fixes: ac2614d721de ("crypto: sun8i-ss - Add support for the PRNG") > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c > index 08a1473b2145..0573f6289e8b 100644 > --- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c > +++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c > @@ -103,7 +103,8 @@ int sun8i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src, > dma_iv = dma_map_single(ss->dev, ctx->seed, ctx->slen, DMA_TO_DEVICE); > if (dma_mapping_error(ss->dev, dma_iv)) { > dev_err(ss->dev, "Cannot DMA MAP IV\n"); > - return -EFAULT; > + err = -EFAULT; > + goto err_d; > } > > dma_dst = dma_map_single(ss->dev, d, todo, DMA_FROM_DEVICE); > @@ -160,7 +161,7 @@ int sun8i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src, > dma_unmap_single(ss->dev, dma_dst, todo, DMA_FROM_DEVICE); > err_iv: > dma_unmap_single(ss->dev, dma_iv, ctx->slen, DMA_TO_DEVICE); > - > +err_d: > if (!err) { > memcpy(dst, d, dlen); > /* Update seed */ > -- > 2.27.0 > Hello The label could be better placed just before the kfree. In error case, there are no need to memzero the not used "d". But a patch with it, was already sent to the ML. Anyway if you fix it, you could add "Acked-by: Corentin Labbe <clabbe.montjoie@gmail.com>" Regards
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c index 08a1473b2145..0573f6289e8b 100644 --- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c +++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c @@ -103,7 +103,8 @@ int sun8i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src, dma_iv = dma_map_single(ss->dev, ctx->seed, ctx->slen, DMA_TO_DEVICE); if (dma_mapping_error(ss->dev, dma_iv)) { dev_err(ss->dev, "Cannot DMA MAP IV\n"); - return -EFAULT; + err = -EFAULT; + goto err_d; } dma_dst = dma_map_single(ss->dev, d, todo, DMA_FROM_DEVICE); @@ -160,7 +161,7 @@ int sun8i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src, dma_unmap_single(ss->dev, dma_dst, todo, DMA_FROM_DEVICE); err_iv: dma_unmap_single(ss->dev, dma_iv, ctx->slen, DMA_TO_DEVICE); - +err_d: if (!err) { memcpy(dst, d, dlen); /* Update seed */
Set _err_ to the return error code -EFAULT before jumping to the new label err_d, so resources for _d_ can be released before returning from function sun8i_ss_prng_generate(). Addresses-Coverity-ID: 1497459 ("Resource leak") Fixes: ac2614d721de ("crypto: sun8i-ss - Add support for the PRNG") Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/crypto/allwinner/sun8i-ss/sun8i-ss-prng.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)