linux-kernel - RE: [EXT] [PATCH v2] crypto: caam: Clear some memory in instantiate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM0PR04MB600410A6F69AB790A3646B6BE7819@AM0PR04MB6004.eurprd04.prod.outlook.com>
Date:   Tue, 21 Mar 2023 07:08:14 +0000
From:   Gaurav Jain <gaurav.jain@....com>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Horia Geanta <horia.geanta@....com>,
        Pankaj Gupta <pankaj.gupta@....com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kim.phillips@...escale.com" <kim.phillips@...escale.com>
CC:     "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: RE: [EXT] [PATCH v2] crypto: caam: Clear some memory in
 instantiate_rng()

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

> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> Sent: Tuesday, March 21, 2023 12:30 PM
> To: Horia Geanta <horia.geanta@....com>; Pankaj Gupta
> <pankaj.gupta@....com>; Gaurav Jain <gaurav.jain@....com>;
> herbert@...dor.apana.org.au; davem@...emloft.net;
> kim.phillips@...escale.com
> Cc: linux-crypto@...r.kernel.org; linux-kernel@...r.kernel.org; kernel-
> janitors@...r.kernel.org; Christophe JAILLET <christophe.jaillet@...adoo.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@...adoo.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