* [PATCH v3] keys/trusted/tpm2: Validate TPM2_Create object sizes separately
@ 2026-09-26 18:42 Srish Srinivasan
2026-09-29 21:15 ` Jarkko Sakkinen
0 siblings, 1 reply; 3+ messages in thread
From: Srish Srinivasan @ 2026-09-26 18:42 UTC (permalink / raw)
To: linux-integrity, keyrings
Cc: James.Bottomley, jarkko, zohar, stefanb, linux-kernel,
linux-security-module, nayna, rnsastry, ssrish
TPM2_Create returns outPrivate, outPublic, creationData, creationHash and
creationTicket in its response parameter area. However, only outPrivate and
outPublic are included in the trusted key blob. The size of the blob is
therefore not determined by the size of the complete response parameter
area.
tpm2_seal_trusted() currently compares the size of the complete response
parameter area against MAX_BLOB_SIZE. This can reject a valid response
when the remaining response outputs cause the entire response parameter
area to exceed MAX_BLOB_SIZE, even though the outPrivate and outPublic
TPM2B structures consumed by tpm2_key_encode() remain small enough to be
encoded in the key blob.
This is observed when creating larger trusted keys using the swtpm TPM 2.0
emulator backed by libtpms.
For example, requesting a 113-byte key succeeds, 114 fails.
~$ keyctl add trusted trusted_key1 "new 113 keyhandle=0x81000001" @u
520504613
~$ keyctl add trusted trusted_key2 "new 114 keyhandle=0x81000001" @u
add_key: Argument list too long
~$
Remove the MAX_BLOB_SIZE check on the complete response parameter area.
Instead, use the response length passed to tpm2_key_encode() to validate
that the outPrivate and outPublic TPM2B structures are fully contained
in the response before accessing them.
Previously, a response parameter area larger than MAX_BLOB_SIZE was
rejected with -E2BIG before ASN.1 encoding. With this change, if the
resulting encoded blob does not fit in payload->blob, the error returned by
asn1_encode_sequence() is propagated instead.
Also, use scope-based cleanup to simplify resource management.
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
---
Changelog:
v3:
- Simplify the TPM2B_PRIVATE and TPM2B_PUBLIC bounds checks
- Use appropriate error codes for failure returns
- Use scope-based cleanup to simplify resource management
v2:
- Exclude a comment pointed out by Jarkko
security/keys/trusted-keys/trusted_tpm2.c | 42 +++++++++++++----------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 906700c3d7f0..920f45c7864b 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -25,24 +25,36 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
{
struct trusted_key_tpm *private = options->private;
const int SCRATCH_SIZE = PAGE_SIZE;
- u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
- u8 *work = scratch, *work1;
- u8 *end_work = scratch + SCRATCH_SIZE;
+ u8 *scratch __free(kfree) = NULL;
+ u8 *work, *work1;
+ u8 *end_work;
u8 *priv, *pub;
- u16 priv_len, pub_len;
+ u32 priv_len, pub_len;
int ret;
+ if (len < 4)
+ return -EINVAL;
+
priv_len = get_unaligned_be16(src) + 2;
- priv = src;
+ if (priv_len + 2 > len)
+ return -EIO;
+ priv = src;
src += priv_len;
pub_len = get_unaligned_be16(src) + 2;
+ if (pub_len + priv_len > len)
+ return -EIO;
+
pub = src;
+ scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
if (!scratch)
return -ENOMEM;
+ work = scratch;
+ end_work = scratch + SCRATCH_SIZE;
+
work = asn1_encode_oid(work, end_work, tpm2key_oid,
asn1_oid_len(tpm2key_oid));
@@ -50,10 +62,9 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
unsigned char bool[3], *w = bool;
/* tag 0 is emptyAuth */
w = asn1_encode_boolean(w, w + sizeof(bool), true);
- if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) {
- ret = PTR_ERR(w);
- goto err;
- }
+ if (WARN(IS_ERR(w), "BUG: Boolean failed to encode"))
+ return PTR_ERR(w);
+
work = asn1_encode_tag(work, end_work, 0, bool, w - bool);
}
@@ -65,8 +76,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
*/
if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE,
"BUG: scratch buffer is too small")) {
- ret = -EINVAL;
- goto err;
+ return -EINVAL;
}
work = asn1_encode_integer(work, end_work, private->keyhandle);
@@ -79,15 +89,10 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
if (IS_ERR(work1)) {
ret = PTR_ERR(work1);
pr_err("BUG: ASN.1 encoder failed with %d\n", ret);
- goto err;
+ return ret;
}
- kfree(scratch);
return work1 - payload->blob;
-
-err:
- kfree(scratch);
- return ret;
}
struct tpm2_key_context {
@@ -340,10 +345,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
blob_len = tpm_buf_read_u32(buf, &offset);
- if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) {
+ if (buf->flags & TPM_BUF_INVALID) {
rc = -E2BIG;
goto out;
}
+
if (buf->length - offset < blob_len) {
rc = -EFAULT;
goto out;
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] keys/trusted/tpm2: Validate TPM2_Create object sizes separately
2026-09-26 18:42 [PATCH v3] keys/trusted/tpm2: Validate TPM2_Create object sizes separately Srish Srinivasan
@ 2026-09-29 21:15 ` Jarkko Sakkinen
2026-09-30 5:52 ` Srish Srinivasan
0 siblings, 1 reply; 3+ messages in thread
From: Jarkko Sakkinen @ 2026-09-29 21:15 UTC (permalink / raw)
To: Srish Srinivasan
Cc: linux-integrity, keyrings, James.Bottomley, zohar, stefanb,
linux-kernel, linux-security-module, nayna, rnsastry
On Sun, Sep 27, 2026 at 12:12:03AM +0530, Srish Srinivasan wrote:
> TPM2_Create returns outPrivate, outPublic, creationData, creationHash and
> creationTicket in its response parameter area. However, only outPrivate and
> outPublic are included in the trusted key blob. The size of the blob is
> therefore not determined by the size of the complete response parameter
> area.
>
> tpm2_seal_trusted() currently compares the size of the complete response
> parameter area against MAX_BLOB_SIZE. This can reject a valid response
> when the remaining response outputs cause the entire response parameter
> area to exceed MAX_BLOB_SIZE, even though the outPrivate and outPublic
> TPM2B structures consumed by tpm2_key_encode() remain small enough to be
> encoded in the key blob.
>
> This is observed when creating larger trusted keys using the swtpm TPM 2.0
> emulator backed by libtpms.
>
> For example, requesting a 113-byte key succeeds, 114 fails.
>
> ~$ keyctl add trusted trusted_key1 "new 113 keyhandle=0x81000001" @u
> 520504613
> ~$ keyctl add trusted trusted_key2 "new 114 keyhandle=0x81000001" @u
> add_key: Argument list too long
> ~$
>
> Remove the MAX_BLOB_SIZE check on the complete response parameter area.
> Instead, use the response length passed to tpm2_key_encode() to validate
> that the outPrivate and outPublic TPM2B structures are fully contained
> in the response before accessing them.
>
> Previously, a response parameter area larger than MAX_BLOB_SIZE was
> rejected with -E2BIG before ASN.1 encoding. With this change, if the
> resulting encoded blob does not fit in payload->blob, the error returned by
> asn1_encode_sequence() is propagated instead.
>
> Also, use scope-based cleanup to simplify resource management.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> ---
> Changelog:
>
> v3:
> - Simplify the TPM2B_PRIVATE and TPM2B_PUBLIC bounds checks
> - Use appropriate error codes for failure returns
> - Use scope-based cleanup to simplify resource management
>
> v2:
> - Exclude a comment pointed out by Jarkko
>
> security/keys/trusted-keys/trusted_tpm2.c | 42 +++++++++++++----------
> 1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 906700c3d7f0..920f45c7864b 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -25,24 +25,36 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> {
> struct trusted_key_tpm *private = options->private;
> const int SCRATCH_SIZE = PAGE_SIZE;
> - u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
> - u8 *work = scratch, *work1;
> - u8 *end_work = scratch + SCRATCH_SIZE;
> + u8 *scratch __free(kfree) = NULL;
> + u8 *work, *work1;
> + u8 *end_work;
> u8 *priv, *pub;
> - u16 priv_len, pub_len;
> + u32 priv_len, pub_len;
> int ret;
>
> + if (len < 4)
> + return -EINVAL;
> +
> priv_len = get_unaligned_be16(src) + 2;
> - priv = src;
> + if (priv_len + 2 > len)
> + return -EIO;
>
> + priv = src;
> src += priv_len;
>
> pub_len = get_unaligned_be16(src) + 2;
> + if (pub_len + priv_len > len)
> + return -EIO;
> +
> pub = src;
>
> + scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
> if (!scratch)
> return -ENOMEM;
>
> + work = scratch;
> + end_work = scratch + SCRATCH_SIZE;
> +
> work = asn1_encode_oid(work, end_work, tpm2key_oid,
> asn1_oid_len(tpm2key_oid));
>
> @@ -50,10 +62,9 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> unsigned char bool[3], *w = bool;
> /* tag 0 is emptyAuth */
> w = asn1_encode_boolean(w, w + sizeof(bool), true);
> - if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) {
> - ret = PTR_ERR(w);
> - goto err;
> - }
> + if (WARN(IS_ERR(w), "BUG: Boolean failed to encode"))
> + return PTR_ERR(w);
> +
> work = asn1_encode_tag(work, end_work, 0, bool, w - bool);
> }
>
> @@ -65,8 +76,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> */
> if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE,
> "BUG: scratch buffer is too small")) {
> - ret = -EINVAL;
> - goto err;
> + return -EINVAL;
> }
>
> work = asn1_encode_integer(work, end_work, private->keyhandle);
> @@ -79,15 +89,10 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> if (IS_ERR(work1)) {
> ret = PTR_ERR(work1);
> pr_err("BUG: ASN.1 encoder failed with %d\n", ret);
> - goto err;
> + return ret;
> }
>
> - kfree(scratch);
> return work1 - payload->blob;
> -
> -err:
> - kfree(scratch);
> - return ret;
> }
>
> struct tpm2_key_context {
> @@ -340,10 +345,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> goto out;
>
> blob_len = tpm_buf_read_u32(buf, &offset);
> - if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) {
> + if (buf->flags & TPM_BUF_INVALID) {
> rc = -E2BIG;
> goto out;
> }
> +
> if (buf->length - offset < blob_len) {
> rc = -EFAULT;
> goto out;
> --
> 2.53.0
>
Now we can say that it leaves the tree to cleaner state than it was
before applying this patch. The first version, despite doing the right
thing was simply too convoluted.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Br, Jarkko
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] keys/trusted/tpm2: Validate TPM2_Create object sizes separately
2026-09-29 21:15 ` Jarkko Sakkinen
@ 2026-09-30 5:52 ` Srish Srinivasan
0 siblings, 0 replies; 3+ messages in thread
From: Srish Srinivasan @ 2026-09-30 5:52 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, keyrings, James.Bottomley, zohar, stefanb,
linux-kernel, linux-security-module, nayna, rnsastry
On 9/30/26 2:45 AM, Jarkko Sakkinen wrote:
> On Sun, Sep 27, 2026 at 12:12:03AM +0530, Srish Srinivasan wrote:
>> TPM2_Create returns outPrivate, outPublic, creationData, creationHash and
>> creationTicket in its response parameter area. However, only outPrivate and
>> outPublic are included in the trusted key blob. The size of the blob is
>> therefore not determined by the size of the complete response parameter
>> area.
>>
>> tpm2_seal_trusted() currently compares the size of the complete response
>> parameter area against MAX_BLOB_SIZE. This can reject a valid response
>> when the remaining response outputs cause the entire response parameter
>> area to exceed MAX_BLOB_SIZE, even though the outPrivate and outPublic
>> TPM2B structures consumed by tpm2_key_encode() remain small enough to be
>> encoded in the key blob.
>>
>> This is observed when creating larger trusted keys using the swtpm TPM 2.0
>> emulator backed by libtpms.
>>
>> For example, requesting a 113-byte key succeeds, 114 fails.
>>
>> ~$ keyctl add trusted trusted_key1 "new 113 keyhandle=0x81000001" @u
>> 520504613
>> ~$ keyctl add trusted trusted_key2 "new 114 keyhandle=0x81000001" @u
>> add_key: Argument list too long
>> ~$
>>
>> Remove the MAX_BLOB_SIZE check on the complete response parameter area.
>> Instead, use the response length passed to tpm2_key_encode() to validate
>> that the outPrivate and outPublic TPM2B structures are fully contained
>> in the response before accessing them.
>>
>> Previously, a response parameter area larger than MAX_BLOB_SIZE was
>> rejected with -E2BIG before ASN.1 encoding. With this change, if the
>> resulting encoded blob does not fit in payload->blob, the error returned by
>> asn1_encode_sequence() is propagated instead.
>>
>> Also, use scope-based cleanup to simplify resource management.
>>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> ---
>> Changelog:
>>
>> v3:
>> - Simplify the TPM2B_PRIVATE and TPM2B_PUBLIC bounds checks
>> - Use appropriate error codes for failure returns
>> - Use scope-based cleanup to simplify resource management
>>
>> v2:
>> - Exclude a comment pointed out by Jarkko
>>
>> security/keys/trusted-keys/trusted_tpm2.c | 42 +++++++++++++----------
>> 1 file changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>> index 906700c3d7f0..920f45c7864b 100644
>> --- a/security/keys/trusted-keys/trusted_tpm2.c
>> +++ b/security/keys/trusted-keys/trusted_tpm2.c
>> @@ -25,24 +25,36 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> {
>> struct trusted_key_tpm *private = options->private;
>> const int SCRATCH_SIZE = PAGE_SIZE;
>> - u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
>> - u8 *work = scratch, *work1;
>> - u8 *end_work = scratch + SCRATCH_SIZE;
>> + u8 *scratch __free(kfree) = NULL;
>> + u8 *work, *work1;
>> + u8 *end_work;
>> u8 *priv, *pub;
>> - u16 priv_len, pub_len;
>> + u32 priv_len, pub_len;
>> int ret;
>>
>> + if (len < 4)
>> + return -EINVAL;
>> +
>> priv_len = get_unaligned_be16(src) + 2;
>> - priv = src;
>> + if (priv_len + 2 > len)
>> + return -EIO;
>>
>> + priv = src;
>> src += priv_len;
>>
>> pub_len = get_unaligned_be16(src) + 2;
>> + if (pub_len + priv_len > len)
>> + return -EIO;
>> +
>> pub = src;
>>
>> + scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
>> if (!scratch)
>> return -ENOMEM;
>>
>> + work = scratch;
>> + end_work = scratch + SCRATCH_SIZE;
>> +
>> work = asn1_encode_oid(work, end_work, tpm2key_oid,
>> asn1_oid_len(tpm2key_oid));
>>
>> @@ -50,10 +62,9 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> unsigned char bool[3], *w = bool;
>> /* tag 0 is emptyAuth */
>> w = asn1_encode_boolean(w, w + sizeof(bool), true);
>> - if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) {
>> - ret = PTR_ERR(w);
>> - goto err;
>> - }
>> + if (WARN(IS_ERR(w), "BUG: Boolean failed to encode"))
>> + return PTR_ERR(w);
>> +
>> work = asn1_encode_tag(work, end_work, 0, bool, w - bool);
>> }
>>
>> @@ -65,8 +76,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> */
>> if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE,
>> "BUG: scratch buffer is too small")) {
>> - ret = -EINVAL;
>> - goto err;
>> + return -EINVAL;
>> }
>>
>> work = asn1_encode_integer(work, end_work, private->keyhandle);
>> @@ -79,15 +89,10 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> if (IS_ERR(work1)) {
>> ret = PTR_ERR(work1);
>> pr_err("BUG: ASN.1 encoder failed with %d\n", ret);
>> - goto err;
>> + return ret;
>> }
>>
>> - kfree(scratch);
>> return work1 - payload->blob;
>> -
>> -err:
>> - kfree(scratch);
>> - return ret;
>> }
>>
>> struct tpm2_key_context {
>> @@ -340,10 +345,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> goto out;
>>
>> blob_len = tpm_buf_read_u32(buf, &offset);
>> - if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) {
>> + if (buf->flags & TPM_BUF_INVALID) {
>> rc = -E2BIG;
>> goto out;
>> }
>> +
>> if (buf->length - offset < blob_len) {
>> rc = -EFAULT;
>> goto out;
>> --
>> 2.53.0
>>
> Now we can say that it leaves the tree to cleaner state than it was
> before applying this patch. The first version, despite doing the right
> thing was simply too convoluted.
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Thanks for the review and for helping improve the patch, Jarkko.
> Br, Jarkko
Thanks,
Srish.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-30 5:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-26 18:42 [PATCH v3] keys/trusted/tpm2: Validate TPM2_Create object sizes separately Srish Srinivasan
2026-09-29 21:15 ` Jarkko Sakkinen
2026-09-30 5:52 ` Srish Srinivasan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®