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

Siddharth Vadapalli <[email protected]>
Newsgroups org.kernel.vger.linux-crypto,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
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.

Fixes: 3bf533787910 ("crypto: sha256 - Use the partial block API")
Cc: <[email protected]>
Signed-off-by: Siddharth Vadapalli <[email protected]>
---
 drivers/crypto/sa2ul.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 965a03d5b27a..984b341755ec 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c
@@ -438,28 +438,28 @@ static void sa_prepare_iopads(struct algo_data *data, const u8 *key,
 	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;
+
+	sha = kmalloc(state_size, GFP_KERNEL);
+	if (!sha)
+		return;
 
 	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);
-
-	sa_export_shash(&sha, shash, digest_size, opad);
+	crypto_shash_update(shash, sha, block_size);
+	sa_export_shash(sha, shash, digest_size, opad);
 
-	memzero_explicit(&sha, sizeof(sha));
+	kfree_sensitive(sha);
 }
 
 /* Derive the inverse key used in AES-CBC decryption operation */
-- 
2.51.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.