Re: [PATCH] keys/trusted/tpm2: Validate TPM2_Create object sizes separately

[Srish Srinivasan <ssrish@linux.ibm.com>]

Hi Jarkko,

On 9/10/26 2:41 AM, Jarkko Sakkinen wrote:
> On Wed, Sep 02, 2026 at 05:13:40PM +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 an 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.
>>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> ---
>>   security/keys/trusted-keys/trusted_tpm2.c | 47 +++++++++++++++++++----
>>   1 file changed, 39 insertions(+), 8 deletions(-)
>>
>> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>> index 67225dd562a9..efb016a6d4b8 100644
>> --- a/security/keys/trusted-keys/trusted_tpm2.c
>> +++ b/security/keys/trusted-keys/trusted_tpm2.c
>> @@ -24,24 +24,54 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>>   			   u8 *src, u32 len)
>>   {
>>   	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;
>> +	u8 *work, *work1;
>> +	u8 *end_work;
>>   	u8 *priv, *pub;
>> -	u16 priv_len, pub_len;
>> +	u32 priv_len, pub_len;
>>   	int ret;
>>   
>> -	priv_len = get_unaligned_be16(src) + 2;
>> +	/*
>> +	 * TPM2_Create Response Parameters:
>> +	 *
>> +	 * outPrivate
>> +	 * outPublic
>> +	 * creationData
>> +	 * creationHash
>> +	 * creationTicket
>> +	 *
>> +	 * Validate outPrivate and outPublic against the response parameter
>> +	 * length before accessing them.
>> +	 */
> Please remove this comment. It is only an annoyance.


Sure, will get rid of it and post v2.

Thanks,
Srish.


>
>> +	if (len < sizeof(__be16))
>> +		return -EFAULT;
>> +
>> +	priv_len = get_unaligned_be16(src);
>> +	if (priv_len > len - sizeof(__be16))
>> +		return -EFAULT;
>> +
>> +	priv_len += sizeof(__be16);
>>   	priv = src;
>>   
>> +	if (len - priv_len < sizeof(__be16))
>> +		return -EFAULT;
>> +
>>   	src += priv_len;
>>   
>> -	pub_len = get_unaligned_be16(src) + 2;
>> +	pub_len = get_unaligned_be16(src);
>> +	if (pub_len > len - priv_len - sizeof(__be16))
>> +		return -EFAULT;
>> +
>> +	pub_len += sizeof(__be16);
>>   	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));
>>   
>> @@ -335,10 +365,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) {
>> -		rc = -E2BIG;
>> +	if (buf->flags & TPM_BUF_INVALID) {
>> +		rc = -EFAULT;
>>   		goto out;
>>   	}
>> +
>>   	if (buf->length - offset < blob_len) {
>>   		rc = -EFAULT;
>>   		goto out;
>> -- 
>> 2.53.0
>>
> BR, Jarkko