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