Re: [PATCH] tpm_tis: Fix undefined behavior in tpm_tis_spi_transfer_half()

[Stefano Garzarella]

On Wed, Sep 17, 2025 at 03:29:56PM +0000, Gunnar Kudrjavets wrote:
When len is 0, the while loop in tpm_tis_spi_transfer_half() never
executes, leaving ret uninitialized. This will lead to undefined
behavior when the function returns.
The issue was introduced when tpm_tis_spi_transfer() was refactored
to call tpm_tis_spi_transfer_half() or tpm_tis_spi_transfer_full().
While ret is properly initialized in tpm_tis_spi_transfer_full(), it
was missed in tpm_tis_spi_transfer_half().

Initialize ret to 0 at the beginning of the function to ensure
defined behavior in all code paths.

Found by GCC 14.2.0 static analyzer with -fanalyzer.

Fixes: a86a42ac2bd6 ("tpm_tis_spi: Add hardware wait polling")
Signed-off-by: Gunnar Kudrjavets <gunna...@amazon.com>
Reviewed-by: Justinien Bouron <jbou...@amazon.com>
---
drivers/char/tpm/tpm_tis_spi_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_tis_spi_main.c 
b/drivers/char/tpm/tpm_tis_spi_main.c
index 61b42c83ced8..1b6d79662ca1 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -85,7 +85,7 @@ static int tpm_tis_spi_transfer_half(struct tpm_tis_data 
*data,       u32 addr,
        struct spi_transfer spi_xfer[3];
        struct spi_message m;
        u8 transfer_len;
-       int ret;
+       int ret = 0;

        while (len) {
                transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);


As an alternative, we can move the declaration in the while block where 
`ret` is always initialized, and we can return 0 at the end of the 
function since all the errors have an early return, I mean something 
like this:

diff --git a/drivers/char/tpm/tpm_tis_spi_main.c 
b/drivers/char/tpm/tpm_tis_spi_main.c
index 61b42c83ced8..c78c0a2d0541 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -85,9 +85,10 @@ static int tpm_tis_spi_transfer_half(struct tpm_tis_data 
*data,            u32 addr,
        struct spi_transfer spi_xfer[3];
        struct spi_message m;
        u8 transfer_len;
-       int ret;

        while (len) {
+               int ret;
+
                transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);

                spi_message_init(&m);
@@ -135,7 +136,7 @@ static int tpm_tis_spi_transfer_half(struct tpm_tis_data 
*data,           u32 addr,
                len -= transfer_len;
        }

-       return ret;
+       return 0;
 }

 static int tpm_tis_spi_transfer_full(struct tpm_tis_data *data, u32 addr,

BTW, I don't have a strong opinion on that, your patch also LGTM:

Reviewed-by: Stefano Garzarella <sgarz...@redhat.com>

Thanks,
Stefano