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

[Jarkko Sakkinen <jarkko@kernel.org>]

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.

> +	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