[PATCH v2] KEYS: encrypted: fix integer overflow of datablob_len

"Cen Zhang (Microsoft Security FORGE Labs)" <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.kernel.lsm
Message-ID <[email protected]>
From: "Cen Zhang (Microsoft Security FORGE Labs)" <[email protected]>

The datablob_len field in struct encrypted_key_payload and the local
variable in encrypted_key_alloc() are declared as unsigned short, which
has a maximum value of 65535. The datablob_len is computed as:

  format_len + 1 + strlen(master_desc) + 1 + strlen(datalen) + 1
  + ivsize + 1 + encrypted_datalen

An attacker can create an encrypted key with a very long datalen string
(e.g., 32756 characters of leading zeros followed by "4096", which
kstrtol() happily parses as 4096), and then update it with a very long
master_desc string (~32760 characters). The combined lengths exceed
65535, causing the unsigned short to silently wrap around. This results
in a grossly undersized kzalloc() allocation, and the subsequent
memcpy() in __ekey_init() writes ~32KB past the end of the buffer,
corrupting adjacent slab objects.

Fix this by:
1. Using check_add_overflow() to calculate datablob_len directly into
   its existing unsigned short destination.
2. Checking the total payload length the same way before passing it to
   key_payload_reserve(), since key->datalen is also unsigned short.
3. Using kzalloc_flex() to allocate encrypted_key_payload together with
   its trailing payload_data[] array.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Cc: [email protected]
Assisted-by: GitHub-Copilot:claude-opus-4.6
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <[email protected]>
Signed-off-by: Francis Perron (Akrites SIRT) <[email protected]>
---
Changes in v2:
- Keep datablob_len unchanged and check both unsigned short bounds with
  check_add_overflow().
- Use kzalloc_flex() for the trailing payload_data[] array.
- Correct the attribution and sign-off trailers.

The initial version was discussed off-list and is not publicly archived.

 security/keys/encrypted-keys/encrypted.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 59cb77b237b3..e07092ea301a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -19,6 +19,7 @@
 #include <linux/parser.h>
 #include <linux/string.h>
 #include <linux/err.h>
+#include <linux/overflow.h>
 #include <keys/user-type.h>
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
@@ -579,6 +580,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 {
 	struct encrypted_key_payload *epayload = NULL;
 	unsigned short datablob_len;
+	unsigned short payload_totallen;
 	unsigned short decrypted_datalen;
 	unsigned short payload_datalen;
 	unsigned int encrypted_datalen;
@@ -632,16 +634,22 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 
 	encrypted_datalen = roundup(decrypted_datalen, blksize);
 
-	datablob_len = format_len + 1 + strlen(master_desc) + 1
-	    + strlen(datalen) + 1 + ivsize + 1 + encrypted_datalen;
+	if (check_add_overflow(format_len + 1 + strlen(master_desc) + 1
+			       + strlen(datalen) + 1 + ivsize + 1,
+			       encrypted_datalen, &datablob_len))
+		return ERR_PTR(-EINVAL);
+
+	if (check_add_overflow(datablob_len,
+			       payload_datalen + HASH_SIZE + 1,
+			       &payload_totallen))
+		return ERR_PTR(-EINVAL);
 
-	ret = key_payload_reserve(key, payload_datalen + datablob_len
-				  + HASH_SIZE + 1);
+	ret = key_payload_reserve(key, payload_totallen);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
-	epayload = kzalloc(sizeof(*epayload) + payload_datalen +
-			   datablob_len + HASH_SIZE + 1, GFP_KERNEL);
+	epayload = kzalloc_flex(*epayload, payload_data, payload_totallen,
+				GFP_KERNEL);
 	if (!epayload)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.55.0
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.