[v2] crypto: caam: Clear some memory in instantiate_rng()

Message ID 41a7e41bb5a14f1e6e68a81c16c90e3ad4542ab1.1679381782.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series [v2] crypto: caam: Clear some memory in instantiate_rng() |

Commit Message

Christophe JAILLET March 21, 2023, 6:59 a.m. UTC
  According to the comment at the end of the 'for' loop just a few lines
below, it looks needed to clear 'desc'.

So it should also be cleared for the first iteration.

Move the memset() to the beginning of the loop to be safe.

Fixes: 281922a1d4f5 ("crypto: caam - add support for SEC v5.x RNG4")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v1 --> v2:
   - move the memset() instead of doing s/kmalloc/kzalloc/
   - adding a Fixes tag

v1:
   https://lore.kernel.org/all/16d6bf3bd7a6e96a8262fcd4680e3ccbb5a50478.1679355849.git.christophe.jaillet@wanadoo.fr/

For for loop has been introduceD in commit 1005bccd7a4a ("crypto: caam -
enable instantiation of all RNG4 state handles"). But if 'desc' really
needs to be cleared, the issue was there before (thus the Fixes tag in
the commit log)
---
 drivers/crypto/caam/ctrl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

Comments

Gaurav Jain March 21, 2023, 7:08 a.m. UTC | #1
Reviewed-by: Gaurav Jain <gaurav.jain@nxp.com>

> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Sent: Tuesday, March 21, 2023 12:30 PM
> To: Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>;
> herbert@gondor.apana.org.au; davem@davemloft.net;
> kim.phillips@freescale.com
> Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> janitors@vger.kernel.org; Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Subject: [EXT] [PATCH v2] crypto: caam: Clear some memory in
> instantiate_rng()
> 
> Caution: EXT Email
> 
> According to the comment at the end of the 'for' loop just a few lines below,
> it looks needed to clear 'desc'.
> 
> So it should also be cleared for the first iteration.
> 
> Move the memset() to the beginning of the loop to be safe.
> 
> Fixes: 281922a1d4f5 ("crypto: caam - add support for SEC v5.x RNG4")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> v1 --> v2:
>    - move the memset() instead of doing s/kmalloc/kzalloc/
>    - adding a Fixes tag
> 
> v1:
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k
> ernel.org%2Fall%2F16d6bf3bd7a6e96a8262fcd4680e3ccbb5a50478.16793558
> 49.git.christophe.jaillet%40wanadoo.fr%2F&data=05%7C01%7Cgaurav.jain%
> 40nxp.com%7Ca1c8a2f58318494a475008db29d9d586%7C686ea1d3bc2b4c6f
> a92cd99c5c301635%7C0%7C0%7C638149787790408570%7CUnknown%7CTW
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6Mn0%3D%7C3000%7C%7C%7C&sdata=UruZict6nLO%2FncETIzKtbI7wBET
> K7%2BfcAEzQyxcS5rU%3D&reserved=0
> 
> For for loop has been introduceD in commit 1005bccd7a4a ("crypto: caam -
> enable instantiation of all RNG4 state handles"). But if 'desc' really needs to
> be cleared, the issue was there before (thus the Fixes tag in the commit log)
> ---
>  drivers/crypto/caam/ctrl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> 6278afb951c3..71b14269a997 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -284,6 +284,10 @@ static int instantiate_rng(struct device *ctrldev, int
> state_handle_mask,
>                 const u32 rdsta_if = RDSTA_IF0 << sh_idx;
>                 const u32 rdsta_pr = RDSTA_PR0 << sh_idx;
>                 const u32 rdsta_mask = rdsta_if | rdsta_pr;
> +
> +               /* Clear the contents before using the descriptor */
> +               memset(desc, 0x00, CAAM_CMD_SZ * 7);
> +
>                 /*
>                  * If the corresponding bit is set, this state handle
>                  * was initialized by somebody else, so it's left alone.
> @@ -327,8 +331,6 @@ static int instantiate_rng(struct device *ctrldev, int
> state_handle_mask,
>                 }
> 
>                 dev_info(ctrldev, "Instantiated RNG4 SH%d\n", sh_idx);
> -               /* Clear the contents before recreating the descriptor */
> -               memset(desc, 0x00, CAAM_CMD_SZ * 7);
>         }
> 
>         kfree(desc);
> --
> 2.32.0
  
Herbert Xu March 31, 2023, 9:53 a.m. UTC | #2
On Tue, Mar 21, 2023 at 07:59:30AM +0100, Christophe JAILLET wrote:
> According to the comment at the end of the 'for' loop just a few lines
> below, it looks needed to clear 'desc'.
> 
> So it should also be cleared for the first iteration.
> 
> Move the memset() to the beginning of the loop to be safe.
> 
> Fixes: 281922a1d4f5 ("crypto: caam - add support for SEC v5.x RNG4")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> v1 --> v2:
>    - move the memset() instead of doing s/kmalloc/kzalloc/
>    - adding a Fixes tag
> 
> v1:
>    https://lore.kernel.org/all/16d6bf3bd7a6e96a8262fcd4680e3ccbb5a50478.1679355849.git.christophe.jaillet@wanadoo.fr/
> 
> For for loop has been introduceD in commit 1005bccd7a4a ("crypto: caam -
> enable instantiation of all RNG4 state handles"). But if 'desc' really
> needs to be cleared, the issue was there before (thus the Fixes tag in
> the commit log)
> ---
>  drivers/crypto/caam/ctrl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Patch applied.  Thanks.
  

Patch

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 6278afb951c3..71b14269a997 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -284,6 +284,10 @@  static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
 		const u32 rdsta_if = RDSTA_IF0 << sh_idx;
 		const u32 rdsta_pr = RDSTA_PR0 << sh_idx;
 		const u32 rdsta_mask = rdsta_if | rdsta_pr;
+
+		/* Clear the contents before using the descriptor */
+		memset(desc, 0x00, CAAM_CMD_SZ * 7);
+
 		/*
 		 * If the corresponding bit is set, this state handle
 		 * was initialized by somebody else, so it's left alone.
@@ -327,8 +331,6 @@  static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
 		}
 
 		dev_info(ctrldev, "Instantiated RNG4 SH%d\n", sh_idx);
-		/* Clear the contents before recreating the descriptor */
-		memset(desc, 0x00, CAAM_CMD_SZ * 7);
 	}
 
 	kfree(desc);