Re: [PATCH v2] KEYS: trusted: Reject short TPM2 public areas

[Jarkko Sakkinen <jarkko@kernel.org>]

On Wed, Oct 21, 2026 at 01:19:54AM +0900, Daehyeon Ko wrote:
> tpm2_load_cmd() reads TPMA_OBJECT with get_unaligned_be32(pub + 4), but
> does not require public_len to cover that field.
> 
> A TPM 2.0 ASN.1 key blob with public_len zero makes the read begin at the
> end of the B + 4-byte decoded allocation, causing a four-byte heap
> out-of-bounds read before the TPM command is transmitted. This is reachable
> from an unprivileged add_key() call when TPM trusted keys and a TPM2 device
> are available.
> 
> This affects v5.13-rc1 and later kernels built with CONFIG_TRUSTED_KEYS=y
> and CONFIG_TRUSTED_KEYS_TPM=y when a TPM2 device is present. A KASAN run
> as UID 1000 with no effective capabilities reported:
> 
> BUG: KASAN: slab-out-of-bounds in tpm2_unseal_trusted
> Read of size 4 at addr ffff888106273b48 by task exploit/160
> CPU: 1 UID: 1000 PID: 160 Comm: exploit
> The buggy address belongs to the object at ffff888106273b40
>  which belongs to the cache kmalloc-8 of size 8
> The buggy address is located 0 bytes to the right of
>  allocated 8-byte region [ffff888106273b40, ffff888106273b48)
> 
> TPMT_PUBLIC starts with the two-byte type, two-byte nameAlg and four-byte
> objectAttributes fields. Require public_len to cover all eight bytes before
> reading the attributes.
> 
> The exact input produced the KASAN read in 3/3 fresh boots. The fixed build
> rejected it with -EINVAL and no KASAN report in 3/3 boots; valid TPM 2.0
> ASN.1 and legacy trusted-key loads continued to succeed.
> 
> The issue was found with an LLM-assisted source audit and validated with an
> end-to-end QEMU and swtpm reproducer.

I did not get any response from you so I'll put one more nit here.

I think it would be better swap these two paragraphs with description
how you address the issue, and why -EINVAL is the right return value
here.

> 
> Fixes: e5fb5d2c5a03 ("security: keys: trusted: Make sealed key properly interoperable")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
> Changes in v2:
> - Describe the wrapper as the TPM 2.0 ASN.1 key format.
> - Explain the eight-byte minimum next to the check.
> - Return -EINVAL for a malformed TPMT_PUBLIC instead of -E2BIG.
> - Replace the obsolete Assisted-by trailer with changelog disclosure.
> 
> v1: https://lore.kernel.org/r/20260925095308.3248297-1-4ncienth@gmail.com
> 
>  security/keys/trusted-keys/trusted_tpm2.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 30188a0ec6766..fb45795b53327 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -425,6 +425,13 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  	if (private_len + 2 + public_len + 2 > blob_len)
>  		return -E2BIG;
>  
> +	/*
> +	 * TPMT_PUBLIC starts with type (2), nameAlg (2), and
> +	 * objectAttributes (4), which is read below.
> +	 */
> +	if (public_len < 8)
> +		return -EINVAL;
> +

I don't think that this nails it as well what this protects against as
what I proposed and it does not wrap at 80 chacters.

However, I think what is better here are the sizes in parentheses so
take my comment and add sizes to it (and wrap to 80 characters).

>  	pub = blob + 2 + private_len + 2;
>  	/* key attributes are always at offset 4 */
>  	attrs = get_unaligned_be32(pub + 4);
> 
> base-commit: 722658cac48a6ba49f5506fd74d010ad9dbb2f05
> -- 
> 2.55.0
> 

Br, Jarkko