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

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

[Srish Srinivasan]

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>
---
Changelog:

 v2:
  - Exclude a comment pointed out by Jarkko

 security/keys/trusted-keys/trusted_tpm2.c | 35 +++++++++++++++++------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 01f18bb37047..cbec4f591952 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -24,24 +24,42 @@ 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;
+	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));
 
@@ -336,10 +354,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

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

[Jarkko Sakkinen]

On Thu, Sep 10, 2026 at 03:32:22PM +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>
> ---
> Changelog:
> 
>  v2:
>   - Exclude a comment pointed out by Jarkko
> 
>  security/keys/trusted-keys/trusted_tpm2.c | 35 +++++++++++++++++------
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 01f18bb37047..cbec4f591952 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -24,24 +24,42 @@ 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;
> +	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;

This caused for me some head scrathing tbh.

See:

1. len >= 2
2. priv_len <= len - 2
3. 2 - priv_len + priv_len <= 2 - priv_len + len - 2 
4. 2 <= len - priv_len

Is this check required?

> +
>  	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));
>  
> @@ -336,10 +354,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

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

[Srish Srinivasan]

On 9/18/26 7:40 AM, Jarkko Sakkinen wrote:
> On Thu, Sep 10, 2026 at 03:32:22PM +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>
>> ---
>> Changelog:
>>
>>   v2:
>>    - Exclude a comment pointed out by Jarkko
>>
>>   security/keys/trusted-keys/trusted_tpm2.c | 35 +++++++++++++++++------
>>   1 file changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>> index 01f18bb37047..cbec4f591952 100644
>> --- a/security/keys/trusted-keys/trusted_tpm2.c
>> +++ b/security/keys/trusted-keys/trusted_tpm2.c
>> @@ -24,24 +24,42 @@ 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;
>> +	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;
> This caused for me some head scrathing tbh.
>
> See:
>
> 1. len >= 2
> 2. priv_len <= len - 2
> 3. 2 - priv_len + priv_len <= 2 - priv_len + len - 2
> 4. 2 <= len - priv_len
>
> Is this check required?


Hi Jarkko,
Thanks for taking a look.

The preceding check,


     if (priv_len > len - sizeof(__be16))
         return -EFAULT;


ensures that the complete TPM2B_PRIVATE, including its size field,
fits within len. After this check, priv_len is incremented by 2 and
therefore represents the complete TPM2B_PRIVATE length.


The check you pointed out,


     if (len - priv_len < sizeof(__be16))
         return -EFAULT;


then ensures that at least 2 bytes remain for reading the following
TPM2B_PUBLIC size field.


>
>> +
>>   	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));
>>   
>> @@ -336,10 +354,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

Thanks,
Srish.

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

[Jarkko Sakkinen]

On Fri, Sep 18, 2026 at 12:18:25PM +0530, Srish Srinivasan wrote:
> 
> On 9/18/26 7:40 AM, Jarkko Sakkinen wrote:
> > On Thu, Sep 10, 2026 at 03:32:22PM +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>
> > > ---
> > > Changelog:
> > > 
> > >   v2:
> > >    - Exclude a comment pointed out by Jarkko
> > > 
> > >   security/keys/trusted-keys/trusted_tpm2.c | 35 +++++++++++++++++------
> > >   1 file changed, 27 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > > index 01f18bb37047..cbec4f591952 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > > @@ -24,24 +24,42 @@ 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;
> > > +	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;
> > This caused for me some head scrathing tbh.
> > 
> > See:
> > 
> > 1. len >= 2
> > 2. priv_len <= len - 2
> > 3. 2 - priv_len + priv_len <= 2 - priv_len + len - 2
> > 4. 2 <= len - priv_len
> > 
> > Is this check required?
> 
> 
> Hi Jarkko,
> Thanks for taking a look.
> 
> The preceding check,
> 
> 
>     if (priv_len > len - sizeof(__be16))
>         return -EFAULT;
> 
> 
> ensures that the complete TPM2B_PRIVATE, including its size field,
> fits within len. After this check, priv_len is incremented by 2 and
> therefore represents the complete TPM2B_PRIVATE length.
> 
> 
> The check you pointed out,
> 
> 
>     if (len - priv_len < sizeof(__be16))
>         return -EFAULT;
> 
> 
> then ensures that at least 2 bytes remain for reading the following
> TPM2B_PUBLIC size field.

OK, maybe I got something wrong in my deduction but I'm not
still entirely sure whether this is leanest way to do these
checks.

I'll iterate some other remarks so that this can move forward.

I was not either entirely sure why this was done:

-	u16 priv_len, pub_len;
+	u32 priv_len, pub_len;

Why wrong len causes -EFAULT and not -EINVAL? EFAULT is for
memory failures. Probably two that follow should return -EIO.

One more that came to mind:

-	priv_len = get_unaligned_be16(src) + 2;
+	if (len < sizeof(__be16))
+		return -EFAULT;
+
+	priv_len = get_unaligned_be16(src);
+	if (priv_len > len - sizeof(__be16))
+		return -EFAULT;
+
+	priv_len += sizeof(__be16);

Removing the line above is not necessary and neither is the last
the last statement if you just change your check accordingly.


BR, Jarkko

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

[Srish Srinivasan]

Hi Jarkko,
Thanks for taking a look.

On 9/25/26 4:51 PM, Jarkko Sakkinen wrote:
> On Fri, Sep 18, 2026 at 12:18:25PM +0530, Srish Srinivasan wrote:
>> On 9/18/26 7:40 AM, Jarkko Sakkinen wrote:
>>> On Thu, Sep 10, 2026 at 03:32:22PM +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>
>>>> ---
>>>> Changelog:
>>>>
>>>>    v2:
>>>>     - Exclude a comment pointed out by Jarkko
>>>>
>>>>    security/keys/trusted-keys/trusted_tpm2.c | 35 +++++++++++++++++------
>>>>    1 file changed, 27 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>>>> index 01f18bb37047..cbec4f591952 100644
>>>> --- a/security/keys/trusted-keys/trusted_tpm2.c
>>>> +++ b/security/keys/trusted-keys/trusted_tpm2.c
>>>> @@ -24,24 +24,42 @@ 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;
>>>> +	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;
>>> This caused for me some head scrathing tbh.
>>>
>>> See:
>>>
>>> 1. len >= 2
>>> 2. priv_len <= len - 2
>>> 3. 2 - priv_len + priv_len <= 2 - priv_len + len - 2
>>> 4. 2 <= len - priv_len
>>>
>>> Is this check required?
>>
>> Hi Jarkko,
>> Thanks for taking a look.
>>
>> The preceding check,
>>
>>
>>      if (priv_len > len - sizeof(__be16))
>>          return -EFAULT;
>>
>>
>> ensures that the complete TPM2B_PRIVATE, including its size field,
>> fits within len. After this check, priv_len is incremented by 2 and
>> therefore represents the complete TPM2B_PRIVATE length.
>>
>>
>> The check you pointed out,
>>
>>
>>      if (len - priv_len < sizeof(__be16))
>>          return -EFAULT;
>>
>>
>> then ensures that at least 2 bytes remain for reading the following
>> TPM2B_PUBLIC size field.
> OK, maybe I got something wrong in my deduction but I'm not
> still entirely sure whether this is leanest way to do these
> checks.


Sure, I will simplify this. Would the following be better?


     if (len < 4)
       return -EINVAL;

     priv_len = get_unaligned_be16(src) + 2;
     if (priv_len + 2 > len)
       return -EIO;

     priv = src;
     src += priv_len;

     pub_len = get_unaligned_be16(src) + 2;
     if (priv_len + pub_len > len)
       return -EIO;

     pub = src;


The first check ensures that len can contain both TPM2B size fields.
The second verifies that the complete TPM2B_PRIVATE leaves room for the
TPM2B_PUBLIC size field. The final check ensures that both the complete
TPM2B_PRIVATE and TPM2B_PUBLIC structures fit within len.

>
> I'll iterate some other remarks so that this can move forward.
>
> I was not either entirely sure why this was done:
>
> -	u16 priv_len, pub_len;
> +	u32 priv_len, pub_len;


Yes, so the change from u16 to u32 was because the values stored in 
priv_len and
pub_len include both the 16-bit value read from the response and the 
two-byte TPM2B
size field.


>
> Why wrong len causes -EFAULT and not -EINVAL? EFAULT is for
> memory failures. Probably two that follow should return -EIO.


Agreed, I will fix this.


>
> One more that came to mind:
>
> -	priv_len = get_unaligned_be16(src) + 2;
> +	if (len < sizeof(__be16))
> +		return -EFAULT;
> +
> +	priv_len = get_unaligned_be16(src);
> +	if (priv_len > len - sizeof(__be16))
> +		return -EFAULT;
> +
> +	priv_len += sizeof(__be16);
>
> Removing the line above is not necessary and neither is the last
> the last statement if you just change your check accordingly.


Yes, that's right. And I have made this change in the diff
that I have pasted above.

Please let me know if this looks better now.

And, thanks for the feedback.

>
>
> BR, Jarkko


Thanks,
Srish.