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: <20190930121520.1388317-2-arnd@arndb.de>
Date:   Mon, 30 Sep 2019 14:14:34 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Antoine Tenart <antoine.tenart@...tlin.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Pascal van Leeuwen <pvanleeuwen@...imatrix.com>,
        Pascal van Leeuwen <pascalvanl@...il.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Eric Biggers <ebiggers@...gle.com>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH 2/3] crypto: inside-secure - Reduce stack usage

safexcel_aead_setkey() contains three large stack variables, totalling
slightly more than the 1024 byte warning limit:

drivers/crypto/inside-secure/safexcel_cipher.c:303:12: error: stack frame size of 1032 bytes in function 'safexcel_aead_setkey' [-Werror,-Wframe-larger-than=]

The function already contains a couple of dynamic allocations, so it is
likely not performance critical and it can only be called in a context
that allows sleeping, so the easiest workaround is to add change it
to use dynamic allocations. Combining istate and ostate into a single
variable simplifies the allocation at the cost of making it slightly
less readable.

Alternatively, it should be possible to shrink these allocations
as the extra buffers appear to be largely unnecessary, but doing
this would be a much more invasive change.

Fixes: 0e17e3621a28 ("crypto: inside-secure - add support for authenc(hmac(sha*),rfc3686(ctr(aes))) suites")
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
 .../crypto/inside-secure/safexcel_cipher.c    | 53 ++++++++++++-------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index ef51f8c2b473..51a4112aa9bc 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -305,10 +305,10 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 {
 	struct crypto_tfm *tfm = crypto_aead_tfm(ctfm);
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
-	struct safexcel_ahash_export_state istate, ostate;
+	struct safexcel_ahash_export_state *state;
 	struct safexcel_crypto_priv *priv = ctx->priv;
+	struct crypto_aes_ctx *aes;
 	struct crypto_authenc_keys keys;
-	struct crypto_aes_ctx aes;
 	int err = -EINVAL;
 
 	if (crypto_authenc_extractkeys(&keys, key, len) != 0)
@@ -334,7 +334,14 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 			goto badkey_expflags;
 		break;
 	case SAFEXCEL_AES:
-		err = aes_expandkey(&aes, keys.enckey, keys.enckeylen);
+		aes = kzalloc(sizeof(*aes), GFP_KERNEL);
+		if (!aes) {
+			err = -ENOMEM;
+			goto badkey;
+		}
+
+		err = aes_expandkey(aes, keys.enckey, keys.enckeylen);
+		kfree(aes);
 		if (unlikely(err))
 			goto badkey;
 		break;
@@ -347,56 +354,66 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 	    memcmp(ctx->key, keys.enckey, keys.enckeylen))
 		ctx->base.needs_inv = true;
 
+	state = kzalloc(sizeof(struct safexcel_ahash_export_state) * 2, GFP_KERNEL);
+	if (!state) {
+		err = -ENOMEM;
+		goto badkey;
+	}
+
 	/* Auth key */
 	switch (ctx->hash_alg) {
 	case CONTEXT_CONTROL_CRYPTO_ALG_SHA1:
 		if (safexcel_hmac_setkey("safexcel-sha1", keys.authkey,
-					 keys.authkeylen, &istate, &ostate))
-			goto badkey;
+					 keys.authkeylen, &state[0], &state[1]))
+			goto badkey_free;
 		break;
 	case CONTEXT_CONTROL_CRYPTO_ALG_SHA224:
 		if (safexcel_hmac_setkey("safexcel-sha224", keys.authkey,
-					 keys.authkeylen, &istate, &ostate))
-			goto badkey;
+					 keys.authkeylen, &state[0], &state[1]))
+			goto badkey_free;
 		break;
 	case CONTEXT_CONTROL_CRYPTO_ALG_SHA256:
 		if (safexcel_hmac_setkey("safexcel-sha256", keys.authkey,
-					 keys.authkeylen, &istate, &ostate))
-			goto badkey;
+					 keys.authkeylen, &state[0], &state[1]))
+			goto badkey_free;
 		break;
 	case CONTEXT_CONTROL_CRYPTO_ALG_SHA384:
 		if (safexcel_hmac_setkey("safexcel-sha384", keys.authkey,
-					 keys.authkeylen, &istate, &ostate))
-			goto badkey;
+					 keys.authkeylen, &state[0], &state[1]))
+			goto badkey_free;
 		break;
 	case CONTEXT_CONTROL_CRYPTO_ALG_SHA512:
 		if (safexcel_hmac_setkey("safexcel-sha512", keys.authkey,
-					 keys.authkeylen, &istate, &ostate))
-			goto badkey;
+					 keys.authkeylen, &state[0], &state[1]))
+			goto badkey_free;
 		break;
 	default:
 		dev_err(priv->dev, "aead: unsupported hash algorithm\n");
-		goto badkey;
+		goto badkey_free;
 	}
 
 	crypto_aead_set_flags(ctfm, crypto_aead_get_flags(ctfm) &
 				    CRYPTO_TFM_RES_MASK);
 
 	if (priv->flags & EIP197_TRC_CACHE && ctx->base.ctxr_dma &&
-	    (memcmp(ctx->ipad, istate.state, ctx->state_sz) ||
-	     memcmp(ctx->opad, ostate.state, ctx->state_sz)))
+	    (memcmp(ctx->ipad, &state[0].state, ctx->state_sz) ||
+	     memcmp(ctx->opad, &state[1].state, ctx->state_sz)))
 		ctx->base.needs_inv = true;
 
 	/* Now copy the keys into the context */
 	memcpy(ctx->key, keys.enckey, keys.enckeylen);
 	ctx->key_len = keys.enckeylen;
 
-	memcpy(ctx->ipad, &istate.state, ctx->state_sz);
-	memcpy(ctx->opad, &ostate.state, ctx->state_sz);
+	memcpy(ctx->ipad, &state[0].state, ctx->state_sz);
+	memcpy(ctx->opad, &state[1].state, ctx->state_sz);
 
 	memzero_explicit(&keys, sizeof(keys));
+	kfree(state);
+
 	return 0;
 
+badkey_free:
+	kfree(state);
 badkey:
 	crypto_aead_set_flags(ctfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
 badkey_expflags:
-- 
2.20.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ