KEYS: encrypted: fix integer overflow of datablob_len
Subsystems (4)
PATCH v2 1/1 KEYS: encrypted: fix integer overflow of datablob_len
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.
---
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(-)
security/keys/encrypted-keys/encrypted.c +14 -6
@@ -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 );
Signature · 1 line
2.55.0
Jarkko Sakkinen
maintainerQuoted 26 lines · Cen Zhang (Microsoft Security FORGE Labs)
From: "Cen Zhang (Microsoft Security FORGE Labs)" <[email protected]>
The
datablob_lenfield in structencrypted_key_payloadand the local variable inencrypted_key_alloc()are declared as unsigned short, which has a maximum value of 65535. Thedatablob_lenis computed as:
format_len+ 1 +strlen(master_desc)+ 1 + strlen(datalen) + 1
+ ivsize + 1 +encrypted_datalenAn 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 longmaster_descstring (~32760 characters). The combined lengths exceed 65535, causing the unsigned short to silently wrap around. This results in a grossly undersizedkzalloc()allocation, and the subsequentmemcpy()in__ekey_init()writes ~32KB past the end of the buffer, corrupting adjacent slab objects.Fix this by:
1. Usingcheck_add_overflow()to calculatedatablob_lendirectly into its existing unsigned short destination.
2. Checking the total payload length the same way before passing it tokey_payload_reserve(), sincekey->datalenis also unsigned short.
3. Usingkzalloc_flex()to allocateencrypted_key_payloadtogether with its trailing payload_data[] array.
Quoted 5 lines
Fixes 7e70cb497850 ("keys: add new key-type encrypted")Assisted-by GitHub-Copilot:claude-opus-4.6Signed-off-by Francis Perron (Akrites SIRT) <[email protected]>
Please remove "(Microsoft Security FORGE Labs)" and "(Akrites SIRT)"
Quoted 63 lines
---
Changes in v2:
- Keepdatablob_lenunchanged and check both unsigned short bounds withcheck_add_overflow().
- Usekzalloc_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(-)security/keys/encrypted-keys/encrypted.c +14
-6@@ -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 );Signature · 1 line
2.55.0
BR, Jarkko
Jarkko Sakkinen
maintainerQuoted 31 lines · Cen Zhang (Microsoft Security FORGE Labs)
From: "Cen Zhang (Microsoft Security FORGE Labs)" <[email protected]>
The
datablob_lenfield in structencrypted_key_payloadand the local variable inencrypted_key_alloc()are declared as unsigned short, which has a maximum value of 65535. Thedatablob_lenis computed as:
format_len+ 1 +strlen(master_desc)+ 1 + strlen(datalen) + 1
+ ivsize + 1 +encrypted_datalenAn 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 longmaster_descstring (~32760 characters). The combined lengths exceed 65535, causing the unsigned short to silently wrap around. This results in a grossly undersizedkzalloc()allocation, and the subsequentmemcpy()in__ekey_init()writes ~32KB past the end of the buffer, corrupting adjacent slab objects.Fix this by:
1. Usingcheck_add_overflow()to calculatedatablob_lendirectly into its existing unsigned short destination.
2. Checking the total payload length the same way before passing it tokey_payload_reserve(), sincekey->datalenis also unsigned short.
3. Usingkzalloc_flex()to allocateencrypted_key_payloadtogether with its trailing payload_data[] array.Fixes 7e70cb497850 ("keys: add new key-type encrypted")Assisted-by GitHub-Copilot:claude-opus-4.6Signed-off-by Francis Perron (Akrites SIRT) <[email protected]>
Quoted 8 lines
---
Changes in v2:
- Keepdatablob_lenunchanged and check both unsigned short bounds withcheck_add_overflow().
- Usekzalloc_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.
I have no recollection of being part of this discussion.
Quoted 55 lines
security/keys/encrypted-keys/encrypted.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)security/keys/encrypted-keys/encrypted.c +14
-6@@ -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 );Signature · 1 line
2.55.0
BR, Jarkko
Jarkko Sakkinen
maintainerQuoted 6 lines · Cen Zhang (Microsoft Security FORGE Labs)
From: "Cen Zhang (Microsoft Security FORGE Labs)" <[email protected]>
The
datablob_lenfield in structencrypted_key_payloadand the local variable inencrypted_key_alloc()are declared as unsigned short, which has a maximum value of 65535. Thedatablob_lenis computed as:
Quoted 8 lines
format_len+ 1 +strlen(master_desc)+ 1 + strlen(datalen) + 1
+ ivsize + 1 +encrypted_datalenAn 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 longmaster_descstring (~32760 characters). The combined lengths exceed 65535, causing the unsigned short to silently wrap around. This results
Please use concrete sizes as possible. E.g., 16-bit word or u16 would be much less involved than "unsigned short".
Quoted 3 lines
in a grossly undersized
kzalloc()allocation, and the subsequentmemcpy()in__ekey_init()writes ~32KB past the end of the buffer, corrupting adjacent slab objects.
One more: please describe the issue as ergnomic as possible (but still complete). I care neither Alice nor Bob.
Quoted 77 lines
Fix this by:
1. Usingcheck_add_overflow()to calculatedatablob_lendirectly into its existing unsigned short destination.
2. Checking the total payload length the same way before passing it tokey_payload_reserve(), sincekey->datalenis also unsigned short.
3. Usingkzalloc_flex()to allocateencrypted_key_payloadtogether with its trailing payload_data[] array.Fixes 7e70cb497850 ("keys: add new key-type encrypted")Assisted-by GitHub-Copilot:claude-opus-4.6Signed-off-by Francis Perron (Akrites SIRT) <[email protected]>---
Changes in v2:
- Keepdatablob_lenunchanged and check both unsigned short bounds withcheck_add_overflow().
- Usekzalloc_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(-)security/keys/encrypted-keys/encrypted.c +14
-6@@ -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 );Signature · 1 line
2.55.0
BR, Jarkko
Hi Jarkko,
Thanks for the review. I'll simplify the description using u16 and U16_MAX, remove the organization names and the off-list note, and send v3 shortly.
Thanks,
Cen