[PATCH v2] crypto: sa2ul - Fix stack overflow in sa_prepare_iopads

[PATCH v2] crypto: sa2ul - Fix stack overflow in sa_prepare_iopads

[Siddharth Vadapalli]

Using sa2ul for IPSec results in the following KASAN report:
  BUG: KASAN: stack-out-of-bounds in __crypto_sha256_export.isra.0+0x1ac/0x1c4
  Write of size 1 at addr ffff80008df96b08 by task charon-systemd/577
  [...]
  Call trace:
   [...]
   __crypto_sha256_export.isra.0+0x1ac/0x1c4
   crypto_sha256_export+0x14/0x24
   crypto_shash_export+0xe8/0x2e0
   sa_export_shash+0x40/0x110 [sa2ul]
   sa_prepare_iopads+0x264/0x4cc [sa2ul]
   sa_init_sc+0x838/0xa78 [sa2ul]
   sa_aead_setkey.constprop.0+0x3a8/0x6e8 [sa2ul]
   sa_aead_cbc_sha256_setkey+0xac/0xec [sa2ul]
   crypto_aead_setkey+0xa8/0x22c
   aead_geniv_setkey+0x34/0x60
   crypto_aead_setkey+0xa8/0x22c
   esp_init_authenc.constprop.0+0x4c0/0x810
   esp_init_state+0x27c/0x3e0
   [...]
  The buggy address belongs to stack of task charon-systemd/577
   and is located at offset 152 in frame:
   sa_prepare_iopads+0x0/0x4cc [sa2ul]
  This frame has 2 objects:
   [48, 152) 'sha'
   [192, 569) '__shash_desc'

Commit 3bf533787910 ("crypto: sha256 - Use the partial block API") added a
real ".export" function to the arch sha256 shash algorithm, changing its
export format to write "sizeof(struct __sha256_ctx) + 1" (105 bytes).

On the other hand, sa_prepare_iopads() passes a stack-allocated union as
the export destination buffer:
  union {
          struct sha1_state sha1;     /* 92 bytes */
	  struct sha256_state sha256; /* 104 bytes */
	  u8 k_pad[SHA1_BLOCK_SIZE];  /* 64 bytes */
  } sha;                              /* 104 bytes total */
with the size of the union being 104 bytes.

Since crypto_shash_export() writes 105 bytes into this 104-byte union, it
overflows by one byte into the adjacent stack frame.

Hence, fix this by replacing the fixed-size stack union with a heap
allocation of crypto_shash_statesize() bytes. Since the contents of the
allocated heap are written-to before they are read, a kmalloc() is safe.

While at it, verify that the size of the authentication key is within the
expected limit.

Fixes: 3bf533787910 ("crypto: sha256 - Use the partial block API")
Cc: <stable@vger.kernel.org>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

Patch is based on commit
cee9395acd80 Linux 7.3-rc1
of Mainline Linux.

v1 of this patch is at:
https://lore.kernel.org/r/20260819142357.3950463-1-s-vadapalli@ti.com/

Changes since v1:
- Propagated error from sa_prepare_iopads throughout the call chain.
- Added missing check for authentication key size being within limits.
Above changes are based on Sashiko review of the v1 patch at:
https://sashiko.dev/#/patchset/20260819142357.3950463-1-s-vadapalli%40ti.com

Regards,
Siddharth.

 drivers/crypto/sa2ul.c | 63 +++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 9846cbeb3449..13d54f269485 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c
@@ -143,8 +143,8 @@ struct algo_data {
 	bool inv_key;
 	struct sa_tfm_ctx *ctx;
 	bool keyed_mac;
-	void (*prep_iopad)(struct algo_data *algo, const u8 *key,
-			   u16 key_sz, __be32 *ipad, __be32 *opad);
+	int (*prep_iopad)(struct algo_data *algo, const u8 *key,
+			  u16 key_sz, __be32 *ipad, __be32 *opad);
 };
 
 /**
@@ -433,34 +433,42 @@ static void sa_export_shash(void *state, struct shash_desc *hash,
 	cpu_to_be32_array(out, result, digest_size / 4);
 }
 
-static void sa_prepare_iopads(struct algo_data *data, const u8 *key,
-			      u16 key_sz, __be32 *ipad, __be32 *opad)
+static int sa_prepare_iopads(struct algo_data *data, const u8 *key,
+			     u16 key_sz, __be32 *ipad, __be32 *opad)
 {
 	SHASH_DESC_ON_STACK(shash, data->ctx->shash);
 	int block_size = crypto_shash_blocksize(data->ctx->shash);
 	int digest_size = crypto_shash_digestsize(data->ctx->shash);
-	union {
-		struct sha1_state sha1;
-		struct sha256_state sha256;
-		u8 k_pad[SHA1_BLOCK_SIZE];
-	} sha;
+	int state_size = crypto_shash_statesize(data->ctx->shash);
+	u8 *sha;
+
+	if (key_sz > block_size) {
+		dev_err(sa_k3_dev, "%s: key size: %u exceeds block size: %d\n",
+			__func__, key_sz, block_size);
+		return -EINVAL;
+	}
+
+	sha = kmalloc(state_size, GFP_KERNEL);
+	if (!sha)
+		return -ENOMEM;
 
 	shash->tfm = data->ctx->shash;
 
-	prepare_kipad(sha.k_pad, key, key_sz);
+	prepare_kipad(sha, key, key_sz);
 
 	crypto_shash_init(shash);
-	crypto_shash_update(shash, sha.k_pad, block_size);
-	sa_export_shash(&sha, shash, digest_size, ipad);
+	crypto_shash_update(shash, sha, block_size);
+	sa_export_shash(sha, shash, digest_size, ipad);
 
-	prepare_kopad(sha.k_pad, key, key_sz);
+	prepare_kopad(sha, key, key_sz);
 
 	crypto_shash_init(shash);
-	crypto_shash_update(shash, sha.k_pad, block_size);
+	crypto_shash_update(shash, sha, block_size);
+	sa_export_shash(sha, shash, digest_size, opad);
 
-	sa_export_shash(&sha, shash, digest_size, opad);
+	kfree_sensitive(sha);
 
-	memzero_explicit(&sha, sizeof(sha));
+	return 0;
 }
 
 /* Derive the inverse key used in AES-CBC decryption operation */
@@ -530,8 +538,8 @@ static int sa_set_sc_enc(struct algo_data *ad, const u8 *key, u16 key_sz,
 }
 
 /* Set Security context for the authentication engine */
-static void sa_set_sc_auth(struct algo_data *ad, const u8 *key, u16 key_sz,
-			   u8 *sc_buf)
+static int sa_set_sc_auth(struct algo_data *ad, const u8 *key, u16 key_sz,
+			  u8 *sc_buf)
 {
 	__be32 *ipad = (void *)(sc_buf + 32);
 	__be32 *opad = (void *)(sc_buf + 64);
@@ -544,11 +552,12 @@ static void sa_set_sc_auth(struct algo_data *ad, const u8 *key, u16 key_sz,
 
 	/* Copy the keys or ipad/opad */
 	if (ad->keyed_mac)
-		ad->prep_iopad(ad, key, key_sz, ipad, opad);
-	else {
-		/* basic hash */
-		sc_buf[1] |= SA_BASIC_HASH;
-	}
+		return ad->prep_iopad(ad, key, key_sz, ipad, opad);
+
+	/* basic hash */
+	sc_buf[1] |= SA_BASIC_HASH;
+
+	return 0;
 }
 
 static inline void sa_copy_iv(__be32 *out, const u8 *iv, bool size16)
@@ -763,9 +772,11 @@ int sa_init_sc(struct sa_ctx_info *ctx, const struct sa_match_data *match_data,
 	}
 
 	/* Prepare context for authentication engine */
-	if (ad->auth_eng.sc_size)
-		sa_set_sc_auth(ad, auth_key, auth_key_sz,
-			       &sc_buf[auth_sc_offset]);
+	if (ad->auth_eng.sc_size) {
+		if (sa_set_sc_auth(ad, auth_key, auth_key_sz,
+				   &sc_buf[auth_sc_offset]))
+			return -EINVAL;
+	}
 
 	/* Set the ownership of context to CP_ACE */
 	sc_buf[SA_CTX_SCCTL_OWNER_OFFSET] = 0x80;
-- 
2.51.1