* [PATCH] crypto: caam/qi2: remove redundant assignment to ret @ 2020-06-11 15:39 Colin King 2020-06-18 7:58 ` Herbert Xu 2020-06-18 10:54 ` Horia Geantă 0 siblings, 2 replies; 9+ messages in thread From: Colin King @ 2020-06-11 15:39 UTC (permalink / raw) To: Horia Geantă, Aymen Sghaier, Herbert Xu, David S . Miller, linux-crypto Cc: kernel-janitors, linux-kernel From: Colin Ian King <colin.king@canonical.com> The variable ret is being assigned a value that is never read, the error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning ret with -ENOMEM is redundamt. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King <colin.king@canonical.com> --- drivers/crypto/caam/caamalg_qi2.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c index 28669cbecf77..ef2c4e095db3 100644 --- a/drivers/crypto/caam/caamalg_qi2.c +++ b/drivers/crypto/caam/caamalg_qi2.c @@ -4044,7 +4044,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) DMA_TO_DEVICE); if (dma_mapping_error(ctx->dev, edesc->qm_sg_dma)) { dev_err(ctx->dev, "unable to map S/G table\n"); - ret = -ENOMEM; goto unmap; } edesc->qm_sg_bytes = qm_sg_bytes; @@ -4055,7 +4054,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) if (dma_mapping_error(ctx->dev, state->ctx_dma)) { dev_err(ctx->dev, "unable to map ctx\n"); state->ctx_dma = 0; - ret = -ENOMEM; goto unmap; } -- 2.27.0.rc0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret 2020-06-11 15:39 [PATCH] crypto: caam/qi2: remove redundant assignment to ret Colin King @ 2020-06-18 7:58 ` Herbert Xu 2020-06-18 10:40 ` Horia Geantă 2020-06-18 10:54 ` Horia Geantă 1 sibling, 1 reply; 9+ messages in thread From: Herbert Xu @ 2020-06-18 7:58 UTC (permalink / raw) To: Colin King Cc: Horia Geantă, Aymen Sghaier, David S . Miller, linux-crypto, kernel-janitors, linux-kernel On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The variable ret is being assigned a value that is never read, the > error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning > ret with -ENOMEM is redundamt. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/crypto/caam/caamalg_qi2.c | 2 -- > 1 file changed, 2 deletions(-) Patch applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret 2020-06-18 7:58 ` Herbert Xu @ 2020-06-18 10:40 ` Horia Geantă 2020-06-18 10:44 ` Herbert Xu 0 siblings, 1 reply; 9+ messages in thread From: Horia Geantă @ 2020-06-18 10:40 UTC (permalink / raw) To: Herbert Xu, Colin King Cc: Aymen Sghaier, David S . Miller, linux-crypto, kernel-janitors, linux-kernel On 6/18/2020 10:58 AM, Herbert Xu wrote: > On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> The variable ret is being assigned a value that is never read, the >> error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning >> ret with -ENOMEM is redundamt. >> >> Addresses-Coverity: ("Unused value") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/crypto/caam/caamalg_qi2.c | 2 -- >> 1 file changed, 2 deletions(-) > > Patch applied. Thanks. > Unfortunately I missed this patch, and it doesn't look correct. Do I need to send a revert? Thanks, Horia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret 2020-06-18 10:40 ` Horia Geantă @ 2020-06-18 10:44 ` Herbert Xu 0 siblings, 0 replies; 9+ messages in thread From: Herbert Xu @ 2020-06-18 10:44 UTC (permalink / raw) To: Horia Geantă Cc: Colin King, Aymen Sghaier, David S . Miller, linux-crypto, kernel-janitors, linux-kernel On Thu, Jun 18, 2020 at 01:40:55PM +0300, Horia Geantă wrote: > On 6/18/2020 10:58 AM, Herbert Xu wrote: > > On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote: > >> From: Colin Ian King <colin.king@canonical.com> > >> > >> The variable ret is being assigned a value that is never read, the > >> error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning > >> ret with -ENOMEM is redundamt. > >> > >> Addresses-Coverity: ("Unused value") > >> Signed-off-by: Colin Ian King <colin.king@canonical.com> > >> --- > >> drivers/crypto/caam/caamalg_qi2.c | 2 -- > >> 1 file changed, 2 deletions(-) > > > > Patch applied. Thanks. > > > Unfortunately I missed this patch, and it doesn't look correct. > > Do I need to send a revert? No please check again. The patch is correct. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret 2020-06-11 15:39 [PATCH] crypto: caam/qi2: remove redundant assignment to ret Colin King 2020-06-18 7:58 ` Herbert Xu @ 2020-06-18 10:54 ` Horia Geantă 2020-06-18 11:00 ` Herbert Xu 1 sibling, 1 reply; 9+ messages in thread From: Horia Geantă @ 2020-06-18 10:54 UTC (permalink / raw) To: Colin King, Aymen Sghaier, Herbert Xu, David S . Miller, linux-crypto Cc: kernel-janitors, linux-kernel On 6/11/2020 6:39 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The variable ret is being assigned a value that is never read, the > error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning > ret with -ENOMEM is redundamt. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/crypto/caam/caamalg_qi2.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c > index 28669cbecf77..ef2c4e095db3 100644 > --- a/drivers/crypto/caam/caamalg_qi2.c > +++ b/drivers/crypto/caam/caamalg_qi2.c > @@ -4044,7 +4044,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) > DMA_TO_DEVICE); > if (dma_mapping_error(ctx->dev, edesc->qm_sg_dma)) { > dev_err(ctx->dev, "unable to map S/G table\n"); > - ret = -ENOMEM; > goto unmap; > } > edesc->qm_sg_bytes = qm_sg_bytes; > @@ -4055,7 +4054,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) > if (dma_mapping_error(ctx->dev, state->ctx_dma)) { > dev_err(ctx->dev, "unable to map ctx\n"); > state->ctx_dma = 0; > - ret = -ENOMEM; > goto unmap; > } > The proper fix would be updating the ahash_finup_no_ctx() function to return the specific error code: return ret; instead of returning -ENOMEM for all error cases. For example error code returned by dpaa2_caam_enqueue() should be returned instead of -ENOMEM. Horia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret 2020-06-18 10:54 ` Horia Geantă @ 2020-06-18 11:00 ` Herbert Xu 2020-06-18 13:16 ` Horia Geantă 0 siblings, 1 reply; 9+ messages in thread From: Herbert Xu @ 2020-06-18 11:00 UTC (permalink / raw) To: Horia Geantă Cc: Colin King, Aymen Sghaier, David S . Miller, linux-crypto, kernel-janitors, linux-kernel On Thu, Jun 18, 2020 at 01:54:55PM +0300, Horia Geantă wrote: > > The proper fix would be updating the ahash_finup_no_ctx() function > to return the specific error code: > return ret; > instead of returning -ENOMEM for all error cases. > > For example error code returned by dpaa2_caam_enqueue() > should be returned instead of -ENOMEM. You can do that as a follow-up. The patch is correct as is. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret 2020-06-18 11:00 ` Herbert Xu @ 2020-06-18 13:16 ` Horia Geantă 2020-06-19 13:22 ` [PATCH] crypto: caam/qi2 - fix return code in ahash_finup_no_ctx() Horia Geantă 0 siblings, 1 reply; 9+ messages in thread From: Horia Geantă @ 2020-06-18 13:16 UTC (permalink / raw) To: Herbert Xu Cc: Colin King, Aymen Sghaier, David S . Miller, linux-crypto, kernel-janitors, linux-kernel On 6/18/2020 2:00 PM, Herbert Xu wrote: > On Thu, Jun 18, 2020 at 01:54:55PM +0300, Horia Geantă wrote: >> >> The proper fix would be updating the ahash_finup_no_ctx() function >> to return the specific error code: >> return ret; >> instead of returning -ENOMEM for all error cases. >> >> For example error code returned by dpaa2_caam_enqueue() >> should be returned instead of -ENOMEM. > > You can do that as a follow-up. The patch is correct as is. > Just that the follow-up implies adding all the code back. Anyway, not a big deal... Thanks, Horia ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] crypto: caam/qi2 - fix return code in ahash_finup_no_ctx() 2020-06-18 13:16 ` Horia Geantă @ 2020-06-19 13:22 ` Horia Geantă 2020-06-26 6:08 ` Herbert Xu 0 siblings, 1 reply; 9+ messages in thread From: Horia Geantă @ 2020-06-19 13:22 UTC (permalink / raw) To: Herbert Xu, David S. Miller Cc: Aymen Sghaier, Colin King, linux-crypto, NXP Linux Team ahash_finup_no_ctx() returns -ENOMEM in most error cases, and this is fine for almost all of them. However, the return code provided by dpaa2_caam_enqueue() (e.g. -EIO or -EBUSY) shouldn't be overridden by -ENOMEM. Signed-off-by: Horia Geantă <horia.geanta@nxp.com> --- drivers/crypto/caam/caamalg_qi2.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c index 1e90412afea2..45e9ff851e2d 100644 --- a/drivers/crypto/caam/caamalg_qi2.c +++ b/drivers/crypto/caam/caamalg_qi2.c @@ -4004,7 +4004,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req) int digestsize = crypto_ahash_digestsize(ahash); struct ahash_edesc *edesc; struct dpaa2_sg_entry *sg_table; - int ret; + int ret = -ENOMEM; src_nents = sg_nents_for_len(req->src, req->nbytes); if (src_nents < 0) { @@ -4017,7 +4017,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req) DMA_TO_DEVICE); if (!mapped_nents) { dev_err(ctx->dev, "unable to DMA map source\n"); - return -ENOMEM; + return ret; } } else { mapped_nents = 0; @@ -4027,7 +4027,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req) edesc = qi_cache_zalloc(GFP_DMA | flags); if (!edesc) { dma_unmap_sg(ctx->dev, req->src, src_nents, DMA_TO_DEVICE); - return -ENOMEM; + return ret; } edesc->src_nents = src_nents; @@ -4044,6 +4044,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req) DMA_TO_DEVICE); if (dma_mapping_error(ctx->dev, edesc->qm_sg_dma)) { dev_err(ctx->dev, "unable to map S/G table\n"); + ret = -ENOMEM; goto unmap; } edesc->qm_sg_bytes = qm_sg_bytes; @@ -4054,6 +4055,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req) if (dma_mapping_error(ctx->dev, state->ctx_dma)) { dev_err(ctx->dev, "unable to map ctx\n"); state->ctx_dma = 0; + ret = -ENOMEM; goto unmap; } @@ -4080,7 +4082,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req) unmap: ahash_unmap_ctx(ctx->dev, edesc, req, DMA_FROM_DEVICE); qi_cache_free(edesc); - return -ENOMEM; + return ret; } static int ahash_update_first(struct ahash_request *req) -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: caam/qi2 - fix return code in ahash_finup_no_ctx() 2020-06-19 13:22 ` [PATCH] crypto: caam/qi2 - fix return code in ahash_finup_no_ctx() Horia Geantă @ 2020-06-26 6:08 ` Herbert Xu 0 siblings, 0 replies; 9+ messages in thread From: Herbert Xu @ 2020-06-26 6:08 UTC (permalink / raw) To: Horia Geantă Cc: David S. Miller, Aymen Sghaier, Colin King, linux-crypto, NXP Linux Team On Fri, Jun 19, 2020 at 04:22:53PM +0300, Horia Geantă wrote: > ahash_finup_no_ctx() returns -ENOMEM in most error cases, > and this is fine for almost all of them. > > However, the return code provided by dpaa2_caam_enqueue() > (e.g. -EIO or -EBUSY) shouldn't be overridden by -ENOMEM. > > Signed-off-by: Horia Geantă <horia.geanta@nxp.com> > --- > drivers/crypto/caam/caamalg_qi2.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) Patch applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-26 6:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-11 15:39 [PATCH] crypto: caam/qi2: remove redundant assignment to ret Colin King 2020-06-18 7:58 ` Herbert Xu 2020-06-18 10:40 ` Horia Geantă 2020-06-18 10:44 ` Herbert Xu 2020-06-18 10:54 ` Horia Geantă 2020-06-18 11:00 ` Herbert Xu 2020-06-18 13:16 ` Horia Geantă 2020-06-19 13:22 ` [PATCH] crypto: caam/qi2 - fix return code in ahash_finup_no_ctx() Horia Geantă 2020-06-26 6:08 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).