[PATCH 1/2] tpm: st33zp24: Return zero on status read failure

[PATCH 1/2] tpm: st33zp24: Return zero on status read failure

[Ruoyu Wang]

st33zp24_status() ignores the result of the transport read and returns
data even when no byte was received. The I2C transport, for example,
skips i2c_master_recv() when the register-select write is short or fails,
leaving data uninitialized. The resulting stack value can be interpreted
as TPM_STS flags and let status checks complete spuriously.

The status callback cannot propagate a transport error. Return zero
unless recv() reports exactly one byte. With no status bits set, callers
retry or take their existing timeout or error path instead of acting on
an invalid status value.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: 251a7b08213a ("TPM: STMicroelectronics ST33 I2C KERNEL 3.x")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/char/tpm/st33zp24/st33zp24.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index e2b7451ea7ccd3..898e8d01d26698 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -93,7 +93,9 @@ static u8 st33zp24_status(struct tpm_chip *chip)
 	struct st33zp24_dev *tpm_dev = dev_get_drvdata(&chip->dev);
 	u8 data;
 
-	tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1);
+	if (tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1) != 1)
+		return 0;
+
 	return data;
 }
 
-- 
2.51.0

[PATCH 2/2] tpm: st33zp24: Validate locality read result

[Ruoyu Wang]

check_locality() treats every nonzero transport return as success. SPI
errors remain negative, while the I2C path can convert a negative write
error through its byte-sized status variable. Either result is nonzero
even though the TPM_ACCESS byte can remain unwritten, so indeterminate
ACTIVE_LOCALITY and VALID bits can falsely report an active locality.

Require recv() to return exactly the requested byte before examining
TPM_ACCESS. Transport errors and short reads now report an inactive
locality, while successful reads retain the existing behavior.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: 251a7b08213a ("TPM: STMicroelectronics ST33 I2C KERNEL 3.x")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/char/tpm/st33zp24/st33zp24.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 898e8d01d26698..0e2deff94c3672 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -106,10 +106,10 @@ static bool check_locality(struct tpm_chip *chip)
 {
 	struct st33zp24_dev *tpm_dev = dev_get_drvdata(&chip->dev);
 	u8 data;
-	u8 status;
+	int status;
 
 	status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
-	if (status && (data &
+	if (status == 1 && (data &
 		(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
 		(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
 		return true;
-- 
2.51.0

Re: [PATCH 1/2] tpm: st33zp24: Return zero on status read failure

[Jarkko Sakkinen]

On Thu, Aug 13, 2026 at 11:30:31PM +0800, Ruoyu Wang wrote:
> st33zp24_status() ignores the result of the transport read and returns
> data even when no byte was received. The I2C transport, for example,
> skips i2c_master_recv() when the register-select write is short or fails,
> leaving data uninitialized. The resulting stack value can be interpreted
> as TPM_STS flags and let status checks complete spuriously.
> 
> The status callback cannot propagate a transport error. Return zero
> unless recv() reports exactly one byte. With no status bits set, callers
> retry or take their existing timeout or error path instead of acting on
> an invalid status value.
> 
> This issue was found by a static analysis checker and confirmed by manual
> source review.
> 
> Fixes: 251a7b08213a ("TPM: STMicroelectronics ST33 I2C KERNEL 3.x")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/char/tpm/st33zp24/st33zp24.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
> index e2b7451ea7ccd3..898e8d01d26698 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.c
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -93,7 +93,9 @@ static u8 st33zp24_status(struct tpm_chip *chip)
>  	struct st33zp24_dev *tpm_dev = dev_get_drvdata(&chip->dev);
>  	u8 data;
>  
> -	tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1);
> +	if (tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1) != 1)
> +		return 0;
> +
>  	return data;
>  }
>  
> -- 
> 2.51.0
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

thanks

BR, Jarkko

Re: [PATCH 2/2] tpm: st33zp24: Validate locality read result

[Jarkko Sakkinen]

On Thu, Aug 13, 2026 at 11:30:32PM +0800, Ruoyu Wang wrote:
> check_locality() treats every nonzero transport return as success. SPI
> errors remain negative, while the I2C path can convert a negative write
> error through its byte-sized status variable. Either result is nonzero
> even though the TPM_ACCESS byte can remain unwritten, so indeterminate
> ACTIVE_LOCALITY and VALID bits can falsely report an active locality.
> 
> Require recv() to return exactly the requested byte before examining
> TPM_ACCESS. Transport errors and short reads now report an inactive
> locality, while successful reads retain the existing behavior.
> 
> This issue was found by a static analysis checker and confirmed by manual
> source review.
> 
> Fixes: 251a7b08213a ("TPM: STMicroelectronics ST33 I2C KERNEL 3.x")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/char/tpm/st33zp24/st33zp24.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
> index 898e8d01d26698..0e2deff94c3672 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.c
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -106,10 +106,10 @@ static bool check_locality(struct tpm_chip *chip)
>  {
>  	struct st33zp24_dev *tpm_dev = dev_get_drvdata(&chip->dev);
>  	u8 data;
> -	u8 status;
> +	int status;
>  
>  	status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
> -	if (status && (data &
> +	if (status == 1 && (data &
>  		(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
>  		(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
>  		return true;
> -- 
> 2.51.0
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko