Re: [PATCH] SoC: cros_ec_codec: switch to library API for SHA-256

[Benson Leung <bleung@google.com>]

[-- Attachment #1: Type: text/plain, Size: 3264 bytes --]

Hi Ard,

On Thu, May 14, 2020 at 06:18:47PM +0200, Ard Biesheuvel wrote:
> The CrOS EC codec driver uses SHA-256 explicitly, and not in a
> performance critical manner, so there is really no point in using
> the dynamic SHASH crypto API here. Let's switch to the library API
> instead.
> 
> Cc: Cheng-Yi Chiang <cychiang@chromium.org> 
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Eric Biggers <ebiggers@kernel.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Looking at the code, I was wondering if the SHA-256 is really required
> here? It looks like it is using it as some kind of fingerprint to decide
> whether the provided file is identical to the one that has already been
> loaded. If this is the case, we should probably just use CRC32 instead.
> 
> Also, do we really need to wipe the context struct? Is there any security
> sensitive data in there?
> 

Adding Tzung-Bi Shih <tzungbi@google.com> to help answer these, as these
were added as a part of his change here:
 b6bc07d4360d  ASoC: cros_ec_codec: support WoV


Thanks,
Benson

>  sound/soc/codecs/Kconfig         |  3 +--
>  sound/soc/codecs/cros_ec_codec.c | 21 +++++---------------
>  2 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index e6a0c5d05fa5..c7ce4cc658cf 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -537,8 +537,7 @@ config SND_SOC_CQ0093VC
>  config SND_SOC_CROS_EC_CODEC
>  	tristate "codec driver for ChromeOS EC"
>  	depends on CROS_EC
> -	select CRYPTO
> -	select CRYPTO_SHA256
> +	select CRYPTO_LIB_SHA256
>  	help
>  	  If you say yes here you will get support for the
>  	  ChromeOS Embedded Controller's Audio Codec.
> diff --git a/sound/soc/codecs/cros_ec_codec.c b/sound/soc/codecs/cros_ec_codec.c
> index d3dc42aa6825..6bc02c485ab2 100644
> --- a/sound/soc/codecs/cros_ec_codec.c
> +++ b/sound/soc/codecs/cros_ec_codec.c
> @@ -107,24 +107,13 @@ static int send_ec_host_command(struct cros_ec_device *ec_dev, uint32_t cmd,
>  static int calculate_sha256(struct cros_ec_codec_priv *priv,
>  			    uint8_t *buf, uint32_t size, uint8_t *digest)
>  {
> -	struct crypto_shash *tfm;
> +	struct sha256_state sctx;
>  
> -	tfm = crypto_alloc_shash("sha256", CRYPTO_ALG_TYPE_SHASH, 0);
> -	if (IS_ERR(tfm)) {
> -		dev_err(priv->dev, "can't alloc shash\n");
> -		return PTR_ERR(tfm);
> -	}
> -
> -	{
> -		SHASH_DESC_ON_STACK(desc, tfm);
> -
> -		desc->tfm = tfm;
> -
> -		crypto_shash_digest(desc, buf, size, digest);
> -		shash_desc_zero(desc);
> -	}
> +	sha256_init(&sctx);
> +	sha256_update(&sctx, buf, size);
> +	sha256_final(&sctx, digest);
>  
> -	crypto_free_shash(tfm);
> +	memzero_explicit(&sctx, sizeof(sctx));
>  
>  #ifdef DEBUG
>  	{
> -- 
> 2.17.1
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]