Skip to contentSkip to conversation

crypto: sa2ul: add missing IS_ERR checks

PATCH In review Ethan Tidmore 2 participants 4 messages Feb 16 – Mar 2 Crypto API

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 crypto: sa2ul: add missing IS_ERR checks

The function dmaengine_desc_get_metadata_ptr() can return an error pointer and is not checked for it. Add error pointer checks.

Fixes 7694b6ca649fe ("crypto: sa2ul - Add crypto driver")
---
 drivers/crypto/sa2ul.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/crypto/sa2ul.c +11 -0
index fdc0b2486069..58d41c269d62 100644
@@ -1051,6 +1051,9 @@ static void sa_aes_dma_in_callback(void *data)
 	if (req->iv) {
 		mdptr = (__be32 *)dmaengine_desc_get_metadata_ptr(rxd->tx_in, &pl,
 							       &ml);
+		if (IS_ERR(mdptr))
+			return;
+
 		result = (u32 *)req->iv;
 
 		for (i = 0; i < (rxd->enc_iv_size / 4); i++)
@@ -1272,6 +1275,8 @@ static int sa_run(struct sa_req *req)
 	 * crypto algorithm to be used, data sizes, different keys etc.
 	 */
 	mdptr = (u32 *)dmaengine_desc_get_metadata_ptr(tx_out, &pl, &ml);
+	if (IS_ERR(mdptr))
+		return PTR_ERR(mdptr);
 
 	sa_prepare_tx_desc(mdptr, (sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS *
 				   sizeof(u32))), cmdl, sizeof(sa_ctx->epib),
@@ -1367,6 +1372,9 @@ static void sa_sha_dma_in_callback(void *data)
 	authsize = crypto_ahash_digestsize(tfm);
 
 	mdptr = (__be32 *)dmaengine_desc_get_metadata_ptr(rxd->tx_in, &pl, &ml);
+	if (IS_ERR(mdptr))
+		return;
+
 	result = (u32 *)req->result;
 
 	for (i = 0; i < (authsize / 4); i++)
@@ -1677,6 +1685,9 @@ static void sa_aead_dma_in_callback(void *data)
 	authsize = crypto_aead_authsize(tfm);
 
 	mdptr = (u32 *)dmaengine_desc_get_metadata_ptr(rxd->tx_in, &pl, &ml);
+	if (IS_ERR(mdptr))
+		return;
+
 	for (i = 0; i < (authsize / 4); i++)
 		mdptr[i + 4] = swab32(mdptr[i + 4]);
 
Signature · 1 line
2.53.0
maintainer
#1 lore raw
Quoted 14 lines · Ethan Tidmore

The function dmaengine_desc_get_metadata_ptr() can return an error pointer and is not checked for it. Add error pointer checks.

Fixes 7694b6ca649fe ("crypto: sa2ul - Add crypto driver")
---
 drivers/crypto/sa2ul.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/crypto/sa2ul.c +0 -0
index fdc0b2486069..58d41c269d62 100644

drivers/crypto/sa2ul.c static void sa_aes_dma_in_callback(void *data) · line 1055

Quoted 5 lines
drivers/crypto/sa2ul.c +2 -0
@@ -1051,6 +1051,9 @@ static void sa_aes_dma_in_callback(void *data)
 	if (req->iv) {
 		mdptr = (__be32 *)dmaengine_desc_get_metadata_ptr(rxd->tx_in, &pl,
 							       &ml);
+		if (IS_ERR(mdptr))
+			return;

Thanks for adding the error checks. However, if we get an error here shouldn't we still free the resources and pass the error up to the caller?

Thanks,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#2 lore raw
Quoted 25 lines · Herbert Xu
Quoted 14 lines · Ethan Tidmore

The function dmaengine_desc_get_metadata_ptr() can return an error pointer and is not checked for it. Add error pointer checks.

Fixes 7694b6ca649fe ("crypto: sa2ul - Add crypto driver")
---
 drivers/crypto/sa2ul.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/crypto/sa2ul.c +0 -0
index fdc0b2486069..58d41c269d62 100644

drivers/crypto/sa2ul.c static void sa_aes_dma_in_callback(void *data) · line 1055

Quoted 5 lines
drivers/crypto/sa2ul.c +2 -0
@@ -1051,6 +1051,9 @@ static void sa_aes_dma_in_callback(void *data)
 	if (req->iv) {
 		mdptr = (__be32 *)dmaengine_desc_get_metadata_ptr(rxd->tx_in, &pl,
 							       &ml);
+		if (IS_ERR(mdptr))
+			return;

Thanks for adding the error checks. However, if we get an error here shouldn't we still free the resources and pass the error up to the caller?

Quoted 1 line

Thanks,

Since this is a callback I'm not sure how to propagate the error. Nor am I aware of the specific teardown required for this driver. I'll just drop this patch for now, hopefully I was able to draw your attention to this possible issue

Thanks,

ET

maintainer
#3 lore raw
Quoted 5 lines · Ethan Tidmore

Since this is a callback I'm not sure how to propagate the error. Nor am I aware of the specific teardown required for this driver. I'll just drop this patch for now, hopefully I was able to draw your attention to this possible issue

You can pass the error up by giving it as the second argument to the callback function.

In any case, thanks for reporting this bug.

Cheers,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt