linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
@ 2025-10-03  9:25 Shahriyar Jalayeri
  2025-10-03 11:22 ` Paul Menzel
  2025-10-04 12:29 ` Jarkko Sakkinen
  0 siblings, 2 replies; 8+ messages in thread
From: Shahriyar Jalayeri @ 2025-10-03  9:25 UTC (permalink / raw)
  To: peterhuewe; +Cc: jarkko, jgg, linux-integrity, linux-kernel, Shahriyar Jalayeri

Ensure tpm_inf_recv() does not overflow the provided buffer when
the TPM reports more data than the caller expects.

Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
---
 drivers/char/tpm/tpm_infineon.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 7638b65b8..eb6dd55ff 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -250,6 +250,12 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 	number_of_wtx = 0;
 
 recv_begin:
+	if (count < 4) {
+		dev_err(&chip->dev,
+			"count less than the header size!\n");
+		return -EIO;
+	}
+
 	/* start receiving header */
 	for (i = 0; i < 4; i++) {
 		ret = wait(chip, STAT_RDA);
@@ -268,6 +274,12 @@ 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 > count) {
+			dev_err(&chip->dev,
+				"Buffer too small for incoming data!\n");
+			return -EIO;
+		}
+
 		for (i = 0; i < size; i++) {
 			wait(chip, STAT_RDA);
 			buf[i] = tpm_data_in(RDFIFO);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
  2025-10-03  9:25 Shahriyar Jalayeri
@ 2025-10-03 11:22 ` Paul Menzel
  2025-10-04  8:35   ` Shahriyar
  2025-10-04 12:29 ` Jarkko Sakkinen
  1 sibling, 1 reply; 8+ messages in thread
From: Paul Menzel @ 2025-10-03 11:22 UTC (permalink / raw)
  To: Shahriyar Jalayeri; +Cc: peterhuewe, jarkko, jgg, linux-integrity, linux-kernel

Dear Shahriyar,


Thank you for your patch.

Am 03.10.25 um 11:25 schrieb Shahriyar Jalayeri:
> Ensure tpm_inf_recv() does not overflow the provided buffer when
> the TPM reports more data than the caller expects.

Is it possible to enforce this situation to ensure your patch works?

> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---
>   drivers/char/tpm/tpm_infineon.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..eb6dd55ff 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,12 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>   	number_of_wtx = 0;
>   
>   recv_begin:
> +	if (count < 4) {
> +		dev_err(&chip->dev,
> +			"count less than the header size!\n");

Mention both values count and 4?

> +		return -EIO;
> +	}
> +

This is not described in the commit message.

>   	/* start receiving header */
>   	for (i = 0; i < 4; i++) {
>   		ret = wait(chip, STAT_RDA);
> @@ -268,6 +274,12 @@ 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 > count) {
> +			dev_err(&chip->dev,
> +				"Buffer too small for incoming data!\n");

I’d log both values, and also specify that the operation is aborted.

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


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
  2025-10-03 11:22 ` Paul Menzel
@ 2025-10-04  8:35   ` Shahriyar
  0 siblings, 0 replies; 8+ messages in thread
From: Shahriyar @ 2025-10-04  8:35 UTC (permalink / raw)
  To: Paul Menzel; +Cc: peterhuewe, jarkko, jgg, linux-integrity, linux-kernel

Dear Paul,

> 
> Is it possible to enforce this situation to ensure your patch works?
> 

Triggering the issue requires interposing the hardware or a MCU to 
emulate the TPM and send malformed TPM replies, I might be able to test 
this using Qemu, but that takes some time.

For the rest of your suggestions, I'll send another patch.

BR,
/shj

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
@ 2025-10-04  9:04 Shahriyar Jalayeri
  2025-10-04 12:33 ` Jarkko Sakkinen
  0 siblings, 1 reply; 8+ messages in thread
From: Shahriyar Jalayeri @ 2025-10-04  9:04 UTC (permalink / raw)
  To: Paul Menzel
  Cc: shahriyar, peterhuewe, jarkko, jgg, linux-integrity, linux-kernel

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.

The error messages include both the expected and
actual buffer sizes to indicate that the operation
is aborted.

Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
---
 drivers/char/tpm/tpm_infineon.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 7638b65b8..385bac46a 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -247,11 +247,20 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 	int i;
 	int ret;
 	u32 size = 0;
+	u32 header_size = 4;
 	number_of_wtx = 0;
 
 recv_begin:
+	if (count < header_size) {
+		dev_err(&chip->dev,
+			"Buffer too small (count=%zd, header_size=%u), "
+			"operation aborted\n",
+			count, header_size);
+		return -EIO;
+	}
+
 	/* start receiving header */
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < header_size; i++) {
 		ret = wait(chip, STAT_RDA);
 		if (ret)
 			return -EIO;
@@ -268,6 +277,14 @@ 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 > count) {
+			dev_err(&chip->dev,
+				"Buffer too small for incoming data "
+				"(count=%zd, size=%u), operation aborted\n",
+				count, size);
+			return -EIO;
+		}
+
 		for (i = 0; i < size; i++) {
 			wait(chip, STAT_RDA);
 			buf[i] = tpm_data_in(RDFIFO);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
  2025-10-03  9:25 Shahriyar Jalayeri
  2025-10-03 11:22 ` Paul Menzel
@ 2025-10-04 12:29 ` Jarkko Sakkinen
  1 sibling, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-10-04 12:29 UTC (permalink / raw)
  To: Shahriyar Jalayeri; +Cc: peterhuewe, jgg, linux-integrity, linux-kernel

On Fri, Oct 03, 2025 at 09:25:47AM +0000, Shahriyar Jalayeri wrote:
> Ensure tpm_inf_recv() does not overflow the provided buffer when
> the TPM reports more data than the caller expects.
> 
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>

missing:

Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")

> ---
>  drivers/char/tpm/tpm_infineon.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..eb6dd55ff 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,12 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  	number_of_wtx = 0;
>  
>  recv_begin:
> +	if (count < 4) {
> +		dev_err(&chip->dev,
> +			"count less than the header size!\n");
> +		return -EIO;
> +	}

Please remove dev_err() 

> +
>  	/* start receiving header */
>  	for (i = 0; i < 4; i++) {
>  		ret = wait(chip, STAT_RDA);
> @@ -268,6 +274,12 @@ 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 > count) {
> +			dev_err(&chip->dev,
> +				"Buffer too small for incoming data!\n");
> +			return -EIO;
> +		}


Ditto
>
>  		for (i = 0; i < size; i++) {
>  			wait(chip, STAT_RDA);
>  			buf[i] = tpm_data_in(RDFIFO);
> -- 
> 2.43.0

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
  2025-10-04  9:04 [PATCH] tpm: infineon: add bounds check in tpm_inf_recv Shahriyar Jalayeri
@ 2025-10-04 12:33 ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-10-04 12:33 UTC (permalink / raw)
  To: Shahriyar Jalayeri
  Cc: Paul Menzel, peterhuewe, jgg, linux-integrity, linux-kernel

On Sat, Oct 04, 2025 at 09:04:17AM +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.

What is this 4 byte header? Please describe what it is.

> 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.
> 
> The error messages include both the expected and
> actual buffer sizes to indicate that the operation
> is aborted.

Paragraphs should be 75 chars per line.

> 
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---

Please version your patches e.g., "git format-patch -v2"

And before diff stat it would be good to have a changelog.


>  drivers/char/tpm/tpm_infineon.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..385bac46a 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -247,11 +247,20 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  	int i;
>  	int ret;
>  	u32 size = 0;
> +	u32 header_size = 4;

Please don't use local variable for a constant.


>  	number_of_wtx = 0;
>  
>  recv_begin:
> +	if (count < header_size) {
> +		dev_err(&chip->dev,
> +			"Buffer too small (count=%zd, header_size=%u), "
> +			"operation aborted\n",
> +			count, header_size);
> +		return -EIO;
> +	}
> +
>  	/* start receiving header */
> -	for (i = 0; i < 4; i++) {
> +	for (i = 0; i < header_size; i++) {
>  		ret = wait(chip, STAT_RDA);
>  		if (ret)
>  			return -EIO;
> @@ -268,6 +277,14 @@ 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 > count) {
> +			dev_err(&chip->dev,
> +				"Buffer too small for incoming data "
> +				"(count=%zd, size=%u), operation aborted\n",
> +				count, size);
> +			return -EIO;
> +		}
> +
>  		for (i = 0; i < size; i++) {
>  			wait(chip, STAT_RDA);
>  			buf[i] = tpm_data_in(RDFIFO);
> -- 
> 2.43.0
> 

Other than that, same comments apply as for the previous version.

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
@ 2025-10-07  9:07 Shahriyar Jalayeri
  2025-10-07 19:25 ` Jarkko Sakkinen
  0 siblings, 1 reply; 8+ messages in thread
From: Shahriyar Jalayeri @ 2025-10-07  9:07 UTC (permalink / raw)
  To: jarkko; +Cc: shahriyar, peterhuewe, jgg, linux-integrity, linux-kernel

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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 7638b65b8..8b90a8191 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -250,6 +250,11 @@ 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 +273,10 @@ 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: infineon: add bounds check in tpm_inf_recv
  2025-10-07  9:07 Shahriyar Jalayeri
@ 2025-10-07 19:25 ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-10-07 19:25 UTC (permalink / raw)
  To: Shahriyar Jalayeri; +Cc: peterhuewe, jgg, linux-integrity, linux-kernel

On Tue, Oct 07, 2025 at 09:07:39AM +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 | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..8b90a8191 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,11 @@ 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 +273,10 @@ 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
> 

Nitpick: we don't use curly braces for one line statements.
AFAIK scripts/checkpatch.pl complains about this. Other than that
I don't see any issues.

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-07 19:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-04  9:04 [PATCH] tpm: infineon: add bounds check in tpm_inf_recv Shahriyar Jalayeri
2025-10-04 12:33 ` Jarkko Sakkinen
  -- strict thread matches above, loose matches on Subject: below --
2025-10-07  9:07 Shahriyar Jalayeri
2025-10-07 19:25 ` Jarkko Sakkinen
2025-10-03  9:25 Shahriyar Jalayeri
2025-10-03 11:22 ` Paul Menzel
2025-10-04  8:35   ` Shahriyar
2025-10-04 12:29 ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).