KEYS: trusted: Fix tpm2_load_cmd() boundary check

PATCH In review Jarkko Sakkinen 3 participants 6 messages Sep 1 – Sep 18

Subsystems (4)

Stable branch Security (LSM) Keys-trusted Keys/keyrings

lore

Share & tools

lore

Jarkko Sakkinen

author maintainer

2026-09-01 20:58 UTC #0 lore raw

PATCH 1/1 KEYS: trusted: Fix tpm2_load_cmd() boundary check

tpm2_load_cmd() does boundary checks against the ASN.1 size i.e., payload->blob_len. Address this by passing the decoded blob size to tpm2_load_cmd(), and use it for the boundary checks.

Cc [email protected] # v5.13+

Fixes f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")

Reported-by [email protected]

Closes https://bugs.sh/b/6a581c4284f721d4/

Signed-off-by Jarkko Sakkinen <[email protected]>

---
 security/keys/trusted-keys/trusted_tpm2.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

security/keys/trusted-keys/trusted_tpm2.c +7 -5

index 67225dd562a9..01f18bb37047 100644
@@ -99,7 +99,7 @@ struct tpm2_key_context {
 
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, unsigned int *blob_len)
 {
 	int ret;
 	struct tpm2_key_context ctx;

@@ -120,6 +120,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 		return -ENOMEM;
 
 	*buf = blob;
+	*blob_len = ctx.priv_len + ctx.pub_len;
 	options->keyhandle = ctx.parent;
 
 	memcpy(blob, ctx.priv, ctx.priv_len);

@@ -384,10 +385,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	int rc;
 	u32 attrs;
 
-	rc = tpm2_key_decode(payload, options, &blob);
+	rc = tpm2_key_decode(payload, options, &blob, &blob_len);
 	if (rc) {
 		/* old form */
 		blob = payload->blob;
+		blob_len = payload->blob_len;
 		payload->old_format = 1;
 	} else {
 		/* Bind for cleanup: */

@@ -399,17 +401,17 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		return -EINVAL;
 
 	/* must be big enough for at least the two be16 size counts */
-	if (payload->blob_len < 4)
+	if (blob_len < 4)
 		return -EINVAL;
 
 	private_len = get_unaligned_be16(blob);
 
 	/* must be big enough for following public_len */
-	if (private_len + 2 + 2 > (payload->blob_len))
+	if (private_len + 2 + 2 > blob_len)
 		return -E2BIG;
 
 	public_len = get_unaligned_be16(blob + 2 + private_len);
-	if (private_len + 2 + public_len + 2 > payload->blob_len)
+	if (private_len + 2 + public_len + 2 > blob_len)
 		return -E2BIG;
 
 	pub = blob + 2 + private_len + 2;

Signature · 1 line

2.47.3

Stefano Garzarella

Reviewed-by on v1 1/1

2026-09-02 09:24 UTC #1 lore raw

security/keys/trusted-keys/trusted_tpm2.c static int tpm2_key_decode(struct trusted_key_payload *payload,

Quoted 27 lines · Jarkko Sakkinen

tpm2_load_cmd() does boundary checks against the ASN.1 size i.e., payload->blob_len. Address this by passing the decoded blob size to tpm2_load_cmd(), and use it for the boundary checks.
Cc [email protected] # v5.13+
Fixes f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Reported-by [email protected]
Closes https://bugs.sh/b/6a581c4284f721d4/
Signed-off-by Jarkko Sakkinen <[email protected]>
---
security/keys/trusted-keys/trusted_tpm2.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
security/keys/trusted-keys/trusted_tpm2.c +1 -1
index 67225dd562a9..01f18bb37047 100644
@@ -99,7 +99,7 @@ struct tpm2_key_context {
 
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, unsigned int *blob_len)
 {
 	int ret;
 	struct tpm2_key_context ctx;
@@ -120,6 +120,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,

	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);

Pre-existing, but is + 4 here useless?

I'm not asking to fix here, just noticed while reviewing. Maybe we can set *blob_len earlier and use it also in the kmalloc(). Not a strong opinion, that said this LGTM:

Reviewed-by Stefano Garzarella <[email protected]>

	if (!blob)

Quoted 44 lines

security/keys/trusted-keys/trusted_tpm2.c +6 -4

 		return -ENOMEM;
 
 	*buf = blob;
+	*blob_len = ctx.priv_len + ctx.pub_len;
 	options->keyhandle = ctx.parent;
 
 	memcpy(blob, ctx.priv, ctx.priv_len);

@@ -384,10 +385,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	int rc;
 	u32 attrs;
 
-	rc = tpm2_key_decode(payload, options, &blob);
+	rc = tpm2_key_decode(payload, options, &blob, &blob_len);
 	if (rc) {
 		/* old form */
 		blob = payload->blob;
+		blob_len = payload->blob_len;
 		payload->old_format = 1;
 	} else {
 		/* Bind for cleanup: */

@@ -399,17 +401,17 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		return -EINVAL;
 
 	/* must be big enough for at least the two be16 size counts */
-	if (payload->blob_len < 4)
+	if (blob_len < 4)
 		return -EINVAL;
 
 	private_len = get_unaligned_be16(blob);
 
 	/* must be big enough for following public_len */
-	if (private_len + 2 + 2 > (payload->blob_len))
+	if (private_len + 2 + 2 > blob_len)
 		return -E2BIG;
 
 	public_len = get_unaligned_be16(blob + 2 + private_len);
-	if (private_len + 2 + public_len + 2 > payload->blob_len)
+	if (private_len + 2 + public_len + 2 > blob_len)
 		return -E2BIG;
 
 	pub = blob + 2 + private_len + 2;

Signature · 1 line

2.47.3

Jarkko Sakkinen

author maintainer

2026-09-09 20:59 UTC #2 lore raw

Quoted 31 lines · Stefano Garzarella

security/keys/trusted-keys/trusted_tpm2.c static int tpm2_key_decode(struct trusted_key_payload *payload,
Quoted 27 lines · Jarkko Sakkinen
tpm2_load_cmd() does boundary checks against the ASN.1 size i.e., payload->blob_len. Address this by passing the decoded blob size to tpm2_load_cmd(), and use it for the boundary checks.
Cc [email protected] # v5.13+
Fixes f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Reported-by [email protected]
Closes https://bugs.sh/b/6a581c4284f721d4/
Signed-off-by Jarkko Sakkinen <[email protected]>
---
security/keys/trusted-keys/trusted_tpm2.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
security/keys/trusted-keys/trusted_tpm2.c +1 -1
index 67225dd562a9..01f18bb37047 100644
@@ -99,7 +99,7 @@ struct tpm2_key_context {
 
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, unsigned int *blob_len)
 {
 	int ret;
 	struct tpm2_key_context ctx;
@@ -120,6 +120,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);

Quoted 1 line

Pre-existing, but is + 4 here useless?

I checked this through in detail.

It should not be like that given that callbacks tpm2_key_{pub,priv} provide the length of TPM2B_PUBLIC and TPM2B_PRIVATE.

Thus, it is a bug introduced by f2219745250f ("security: keys: trusted:
use ASN.1 TPM2 key format for the blobs")

BR, Jarkko

Stefano Garzarella

2026-09-10 07:52 UTC #3 lore raw

Quoted 34 lines · Jarkko Sakkinen

Quoted 31 lines · Stefano Garzarella
security/keys/trusted-keys/trusted_tpm2.c static int tpm2_key_decode(struct trusted_key_payload *payload,
Quoted 27 lines · Jarkko Sakkinen
tpm2_load_cmd() does boundary checks against the ASN.1 size i.e., payload->blob_len. Address this by passing the decoded blob size to tpm2_load_cmd(), and use it for the boundary checks.
Cc [email protected] # v5.13+
Fixes f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Reported-by [email protected]
Closes https://bugs.sh/b/6a581c4284f721d4/
Signed-off-by Jarkko Sakkinen <[email protected]>
---
security/keys/trusted-keys/trusted_tpm2.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
security/keys/trusted-keys/trusted_tpm2.c +1 -1
index 67225dd562a9..01f18bb37047 100644
@@ -99,7 +99,7 @@ struct tpm2_key_context {
 
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, unsigned int *blob_len)
 {
 	int ret;
 	struct tpm2_key_context ctx;
@@ -120,6 +120,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);
Quoted 1 line
Pre-existing, but is + 4 here useless?

Quoted 7 lines

I checked this through in detail.
It should not be like that given that callbacks tpm2_key_{pub,priv} provide the length of TPM2B_PUBLIC and TPM2B_PRIVATE.
Thus, it is a bug introduced by f2219745250f ("security: keys: trusted:
use ASN.1 TPM2 key format for the blobs")

Yep, agree. Do you want me to send a fix?

Thanks,
Stefano

Jarkko Sakkinen

author maintainer

2026-09-18 01:43 UTC #4 lore raw

Quoted 37 lines · Stefano Garzarella

Quoted 36 lines · Jarkko Sakkinen
Quoted 31 lines · Stefano Garzarella
security/keys/trusted-keys/trusted_tpm2.c static int tpm2_key_decode(struct trusted_key_payload *payload,
Quoted 27 lines · Jarkko Sakkinen
tpm2_load_cmd() does boundary checks against the ASN.1 size i.e., payload->blob_len. Address this by passing the decoded blob size to tpm2_load_cmd(), and use it for the boundary checks.
Cc [email protected] # v5.13+
Fixes f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Reported-by [email protected]
Closes https://bugs.sh/b/6a581c4284f721d4/
Signed-off-by Jarkko Sakkinen <[email protected]>
---
security/keys/trusted-keys/trusted_tpm2.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
security/keys/trusted-keys/trusted_tpm2.c +1 -1
index 67225dd562a9..01f18bb37047 100644
@@ -99,7 +99,7 @@ struct tpm2_key_context {
 
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, unsigned int *blob_len)
 {
 	int ret;
 	struct tpm2_key_context ctx;
@@ -120,6 +120,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);
Quoted 1 line
Pre-existing, but is + 4 here useless?
I checked this through in detail.

Quoted 7 lines

Quoted 5 lines
It should not be like that given that callbacks tpm2_key_{pub,priv} provide the length of TPM2B_PUBLIC and TPM2B_PRIVATE.
Thus, it is a bug introduced by f2219745250f ("security: keys: trusted:
use ASN.1 TPM2 key format for the blobs")
Yep, agree. Do you want me to send a fix?

Sure, go ahead.

Quoted 2 lines

Thanks,
Stefano

BR, Jarkko

Srish Srinivasan

Tested-by on v1 1/1

2026-09-03 09:31 UTC #5 lore raw

In reply to Jarkko Sakkinen

Quoted 9 lines · Jarkko Sakkinen

tpm2_load_cmd() does boundary checks against the ASN.1 size i.e., payload->blob_len. Address this by passing the decoded blob size to tpm2_load_cmd(), and use it for the boundary checks.
Cc [email protected] # v5.13+
Fixes f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Reported-by [email protected]
Closes https://bugs.sh/b/6a581c4284f721d4/
Signed-off-by Jarkko Sakkinen <[email protected]>

Tested-by Srish Srinivasan <[email protected]>

Quoted 59 lines

---
  security/keys/trusted-keys/trusted_tpm2.c | 12 +++++++-----
  1 file changed, 7 insertions(+), 5 deletions(-)

security/keys/trusted-keys/trusted_tpm2.c +7 -5

index 67225dd562a9..01f18bb37047 100644
@@ -99,7 +99,7 @@ struct tpm2_key_context {
 
  static int tpm2_key_decode(struct trusted_key_payload *payload,
  			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, unsigned int *blob_len)
  {
  	int ret;
  	struct tpm2_key_context ctx;

@@ -120,6 +120,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
  		return -ENOMEM;
 
  	*buf = blob;
+	*blob_len = ctx.priv_len + ctx.pub_len;
  	options->keyhandle = ctx.parent;
 
  	memcpy(blob, ctx.priv, ctx.priv_len);

@@ -384,10 +385,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
  	int rc;
  	u32 attrs;
 
-	rc = tpm2_key_decode(payload, options, &blob);
+	rc = tpm2_key_decode(payload, options, &blob, &blob_len);
  	if (rc) {
  		/* old form */
  		blob = payload->blob;
+		blob_len = payload->blob_len;
  		payload->old_format = 1;
  	} else {
  		/* Bind for cleanup: */

@@ -399,17 +401,17 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
  		return -EINVAL;
 
  	/* must be big enough for at least the two be16 size counts */
-	if (payload->blob_len < 4)
+	if (blob_len < 4)
  		return -EINVAL;
 
  	private_len = get_unaligned_be16(blob);
 
  	/* must be big enough for following public_len */
-	if (private_len + 2 + 2 > (payload->blob_len))
+	if (private_len + 2 + 2 > blob_len)
  		return -E2BIG;
 
  	public_len = get_unaligned_be16(blob + 2 + private_len);
-	if (private_len + 2 + public_len + 2 > payload->blob_len)
+	if (private_len + 2 + public_len + 2 > blob_len)
  		return -E2BIG;
 
  	pub = blob + 2 + private_len + 2;