crypto: inside-secure: safexcel - fix memory allocation

PATCH In review Gustavo A. R. Silva 4 participants 9 messages Oct 8, 2018 – Oct 18, 2018

Subsystems (2)

Crypto API Inside secure crypto driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2018-10-08 19:17 UTC #0 lore raw

PATCH 1/1 crypto: inside-secure: safexcel - fix memory allocation

The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).

Addresses-Coverity-ID: 1473962 ("Sizeof not portable")

Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/inside-secure/safexcel.c +1 -1

index 86c699c1..bc6c5cb 100644
@@ -1066,7 +1066,7 @@ static int safexcel_probe(struct platform_device *pdev)
 
 		priv->ring[i].rdr_req = devm_kcalloc(dev,
 			EIP197_DEFAULT_RING_SIZE,
-			sizeof(priv->ring[i].rdr_req),
+			sizeof(*priv->ring[i].rdr_req),
 			GFP_KERNEL);
 		if (!priv->ring[i].rdr_req) {
 			ret = -ENOMEM;

Signature · 1 line

2.7.4

Kees Cook

Reviewed-by on v1 1/1

2018-10-08 22:20 UTC #1 lore raw

Quoted 7 lines · Gustavo A. R. Silva

The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Luckily, this results in the same size, since it's still a pointer:

struct crypto_async_request **rdr_req;

But yes, it should be fixed.

-Kees

Quoted 20 lines

---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/inside-secure/safexcel.c +1 -2

index 86c699c1..bc6c5cb 100644
@@ -1066,7 +1066,7 @@ static int safexcel_probe(struct platform_device *pdev)
 
                priv->ring[i].rdr_req = devm_kcalloc(dev,
                        EIP197_DEFAULT_RING_SIZE,
-                       sizeof(priv->ring[i].rdr_req),
+                       sizeof(*priv->ring[i].rdr_req),
                        GFP_KERNEL);
                if (!priv->ring[i].rdr_req) {
                        ret = -ENOMEM;
--

2.7.4

Signature · 2 lines

Kees Cook
Pixel Security

Gustavo A. R. Silva

author

2018-10-16 19:44 UTC #2 lore raw

Hi all,

Quoted 6 lines · Kees Cook

Quoted 4 lines · Gustavo A. R. Silva
The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).

Quoted 6 lines

Quoted 3 lines
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Kees Cook <[email protected]>

Friendly ping. Who can take this?

Thanks!
--
Gustavo

Herbert Xu

maintainer

2018-10-17 06:21 UTC #3 lore raw

Quoted 13 lines · Gustavo A. R. Silva

Hi all,
Quoted 10 lines · Kees Cook
Quoted 7 lines · Gustavo A. R. Silva
The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>

Quoted 5 lines

Quoted 2 lines
Reviewed-by Kees Cook <[email protected]>
Friendly ping. Who can take this?

Well I tried to take it but it doesn't apply against cryptodev. So I presume this can go into the tree that carried the change which it depended on?

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

Antoine Tenart

2018-10-17 07:20 UTC #4 lore raw

Hi,

Quoted 16 lines · Herbert Xu

Quoted 15 lines · Gustavo A. R. Silva
Quoted 12 lines · Kees Cook
Quoted 7 lines · Gustavo A. R. Silva
The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Kees Cook <[email protected]>

Quoted 5 lines

Quoted 1 line
Friendly ping. Who can take this?
Well I tried to take it but it doesn't apply against cryptodev. So I presume this can go into the tree that carried the change which it depended on?

I would say this should go in cryptodev. The issue is probably because of other changes that got applied in the meantime. Gustavo can probably rebase his patch on top of cryptodev, and re-send it.

Thanks!
Antoine

Signature · 3 lines

Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Gustavo A. R. Silva

author

2018-10-17 14:41 UTC #5 lore raw

Quoted 21 lines · Antoine Tenart

Hi,
Quoted 18 lines · Herbert Xu
Quoted 16 lines · Gustavo A. R. Silva
Quoted 6 lines · Kees Cook
Quoted 4 lines · Gustavo A. R. Silva
The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).
Quoted 6 lines
Quoted 3 lines
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Kees Cook <[email protected]>
Friendly ping. Who can take this?

Quoted 8 lines

Quoted 3 lines
Well I tried to take it but it doesn't apply against cryptodev. So I presume this can go into the tree that carried the change which it depended on?
I would say this should go in cryptodev. The issue is probably because of other changes that got applied in the meantime. Gustavo can probably rebase his patch on top of cryptodev, and re-send it.

cryptodev is missing the previous commit 329e09893909d409039f6a79757d9b80b67efe39 to which this patch applies.

Kees, did you apply the commit above to your tree?

If so, could you take this patch?

Thanks
--
Gustavo

Kees Cook

2018-10-17 18:23 UTC #6 lore raw

Quoted 33 lines · Gustavo A. R. Silva

Quoted 21 lines · Antoine Tenart
Hi,
Quoted 18 lines · Herbert Xu
Quoted 16 lines · Gustavo A. R. Silva
Quoted 6 lines · Kees Cook
Quoted 4 lines · Gustavo A. R. Silva
The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).
Quoted 6 lines
Quoted 3 lines
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Kees Cook <[email protected]>
Friendly ping. Who can take this?
Quoted 8 lines
Quoted 3 lines
Well I tried to take it but it doesn't apply against cryptodev. So I presume this can go into the tree that carried the change which it depended on?
I would say this should go in cryptodev. The issue is probably because of other changes that got applied in the meantime. Gustavo can probably rebase his patch on top of cryptodev, and re-send it.

Quoted 6 lines

cryptodev is missing the previous commit 329e09893909d409039f6a79757d9b80b67efe39 to which this patch applies.
Kees, did you apply the commit above to your tree?
If so, could you take this patch?

Since this has no functional exposure (the sizes are the same), let's just wait until after the merge window to get this into crypto-next.

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Gustavo A. R. Silva

author

2018-10-18 06:24 UTC #7 lore raw

Quoted 6 lines · Kees Cook

Quoted 2 lines
If so, could you take this patch?
Since this has no functional exposure (the sizes are the same), let's just wait until after the merge window to get this into crypto-next.

Okay. I agree.

Thanks!
--
Gustavo

Antoine Tenart

Acked-by on v1 1/1

2018-10-10 13:55 UTC #8 lore raw

In reply to Gustavo A. R. Silva

Hi Gustavo,

Quoted 7 lines · Gustavo A. R. Silva

The original intention is to allocate space for EIP197_DEFAULT_RING_SIZE *pointers* to struct, so sizeof(priv->ring[i].rdr_req) should be sizeof(*priv->ring[i].rdr_req).
Addresses-Coverity-ID: 1473962 ("Sizeof not portable")
Fixes 9744fec95f06 ("crypto: inside-secure - remove request list to improve performance")
Signed-off-by Gustavo A. R. Silva <[email protected]>

Acked-by Antoine Tenart <[email protected]>

Good catch, thanks!
Antoine

Quoted 20 lines

---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/inside-secure/safexcel.c +1 -1

index 86c699c1..bc6c5cb 100644
@@ -1066,7 +1066,7 @@ static int safexcel_probe(struct platform_device *pdev)
 
 		priv->ring[i].rdr_req = devm_kcalloc(dev,
 			EIP197_DEFAULT_RING_SIZE,
-			sizeof(priv->ring[i].rdr_req),
+			sizeof(*priv->ring[i].rdr_req),
 			GFP_KERNEL);
 		if (!priv->ring[i].rdr_req) {
 			ret = -ENOMEM;

Signature · 1 line

2.7.4

Signature · 3 lines

Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com