[PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv

[PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv

[Shahriyar Jalayeri]

Add two buffer size validations to prevent buffer overflows in
tpm_inf_recv():

1. Validate that the provided buffer can hold at least the 4-byte header
   before attempting to read it.
2. Validate that the buffer is large enough to hold the data size reported
   by the TPM before reading the payload.

Without these checks, a malicious or malfunctioning TPM could cause buffer
overflows by reporting data sizes larger than the provided buffer, leading
to memory corruption.

Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
---
 drivers/char/tpm/tpm_infineon.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 7638b65b8..0fe4193a3 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 	number_of_wtx = 0;
 
 recv_begin:
+    /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
+	if (count < 4)
+		return -EIO;
+
 	/* start receiving header */
 	for (i = 0; i < 4; i++) {
 		ret = wait(chip, STAT_RDA);
@@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 		/* size of the data received */
 		size = ((buf[2] << 8) | buf[3]);
 
+		if (size + 6 > count)
+			return -EIO;
+
 		for (i = 0; i < size; i++) {
 			wait(chip, STAT_RDA);
 			buf[i] = tpm_data_in(RDFIFO);
-- 
2.43.0

Re: [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv

[Paul Menzel]

Dear Shahriyar,


Thank you for the improved version.

Am 10.10.25 um 08:52 schrieb Shahriyar Jalayeri:
> Add two buffer size validations to prevent buffer overflows in
> tpm_inf_recv():
> 
> 1. Validate that the provided buffer can hold at least the 4-byte header
>     before attempting to read it.
> 2. Validate that the buffer is large enough to hold the data size reported
>     by the TPM before reading the payload.
> 
> Without these checks, a malicious or malfunctioning TPM could cause buffer
> overflows by reporting data sizes larger than the provided buffer, leading
> to memory corruption.
> 
> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---

A changelog would be nice to have here.

>   drivers/char/tpm/tpm_infineon.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..0fe4193a3 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>   	number_of_wtx = 0;
>   
>   recv_begin:
> +    /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */

`scripts/checkpatch.pl` should have complained about using spaces 
instead of tabs for indentation.

> +	if (count < 4)
> +		return -EIO;
> +
>   	/* start receiving header */
>   	for (i = 0; i < 4; i++) {
>   		ret = wait(chip, STAT_RDA);
> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>   		/* size of the data received */
>   		size = ((buf[2] << 8) | buf[3]);
>   
> +		if (size + 6 > count)
> +			return -EIO;
> +
>   		for (i = 0; i < size; i++) {
>   			wait(chip, STAT_RDA);
>   			buf[i] = tpm_data_in(RDFIFO);

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv

[Jarkko Sakkinen]

On Fri, Oct 10, 2025 at 06:52:55AM +0000, Shahriyar Jalayeri wrote:
> Add two buffer size validations to prevent buffer overflows in
> tpm_inf_recv():
> 
> 1. Validate that the provided buffer can hold at least the 4-byte header
>    before attempting to read it.
> 2. Validate that the buffer is large enough to hold the data size reported
>    by the TPM before reading the payload.
> 
> Without these checks, a malicious or malfunctioning TPM could cause buffer
> overflows by reporting data sizes larger than the provided buffer, leading
> to memory corruption.
> 
> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---
>  drivers/char/tpm/tpm_infineon.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..0fe4193a3 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  	number_of_wtx = 0;
>  
>  recv_begin:
> +    /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */

This is definitely good enough :-)

But is that comment misaligned? Does VL come from "VLAN"?

> +	if (count < 4)
> +		return -EIO;
> +
>  	/* start receiving header */
>  	for (i = 0; i < 4; i++) {
>  		ret = wait(chip, STAT_RDA);
> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  		/* size of the data received */

I'd delete the above comment.

>  		size = ((buf[2] << 8) | buf[3]);

And use here:

https://elixir.bootlin.com/linux/v6.17.1/source/include/linux/byteorder/generic.h#L108

Not exactly in scope but it would be good convention and make
the check after it more readable.

>  
> +		if (size + 6 > count)
> +			return -EIO;
> +
>  		for (i = 0; i < size; i++) {
>  			wait(chip, STAT_RDA);
>  			buf[i] = tpm_data_in(RDFIFO);
> -- 
> 2.43.0
> 

BR, Jarkko