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

[Gaurav Jain]

Reviewed-by: Gaurav Jain <gaurav.jain@xxxxxxx>

> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> Sent: Tuesday, March 21, 2023 12:30 PM
> To: Horia Geanta <horia.geanta@xxxxxxx>; Pankaj Gupta
> <pankaj.gupta@xxxxxxx>; Gaurav Jain <gaurav.jain@xxxxxxx>;
> herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> kim.phillips@xxxxxxxxxxxxx
> Cc: linux-crypto@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; kernel-
> janitors@xxxxxxxxxxxxxxx; Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> ---
> 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