tpm_i2c_nuvoton: fix tpm_read_fifo overflow check
diff mbox series

Message ID 20200227171505.1793432-1-maurosr@linux.vnet.ibm.com
State Accepted
Headers show
Series
  • tpm_i2c_nuvoton: fix tpm_read_fifo overflow check
Related show

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (82aed17a5468aff6b600ee1694a10a60f942c018)

Commit Message

Mauro S. M. Rodrigues Feb. 27, 2020, 5:15 p.m. UTC
The tpm_read_fifo expects buflen parameter which is the size of buf
parameter. Later it uses buflen to check for an overflow in the case tpm
response is bigger than buf capacity.

The check is fine, but it doesn't interrupt the code flow, so even though
we see error messages about the overflow, it doesn't prevent it.

Adding a goto after specifying the error return code fixes it.

Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>
---
 libstb/drivers/tpm_i2c_nuvoton.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Claudio Carvalho March 3, 2020, 6:15 p.m. UTC | #1
Good catch!

Reviewed-by: Claudio Carvalho <cclaudio@linux.ibm.com>

Claudio

On 2/27/20 2:15 PM, Mauro S. M. Rodrigues wrote:
> The tpm_read_fifo expects buflen parameter which is the size of buf
> parameter. Later it uses buflen to check for an overflow in the case tpm
> response is bigger than buf capacity.
>
> The check is fine, but it doesn't interrupt the code flow, so even though
> we see error messages about the overflow, it doesn't prevent it.
>
> Adding a goto after specifying the error return code fixes it.
>
> Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>
> ---
>  libstb/drivers/tpm_i2c_nuvoton.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libstb/drivers/tpm_i2c_nuvoton.c b/libstb/drivers/tpm_i2c_nuvoton.c
> index 3679ddafc..d668000e4 100644
> --- a/libstb/drivers/tpm_i2c_nuvoton.c
> +++ b/libstb/drivers/tpm_i2c_nuvoton.c
> @@ -383,6 +383,7 @@ static int tpm_read_fifo(uint8_t* buf, size_t* buflen)
>  			prlog(PR_ERR, "NUVOTON: overflow on fifo read, c=%zd, "
>  			      "bc=%d, bl=%zd\n", count, burst_count, *buflen);
>  			rc = STB_TPM_OVERFLOW;
> +			goto error;
>  		}
>  		rc = tpm_i2c_request_send(tpm_device,
>  					  SMBUS_READ,
Klaus Heinrich Kiwi March 6, 2020, 12:52 p.m. UTC | #2
On 2/27/2020 2:15 PM, Mauro S. M. Rodrigues wrote:
> The tpm_read_fifo expects buflen parameter which is the size of buf
> parameter. Later it uses buflen to check for an overflow in the case tpm
> response is bigger than buf capacity.
> 
> The check is fine, but it doesn't interrupt the code flow, so even though
> we see error messages about the overflow, it doesn't prevent it.
> 
> Adding a goto after specifying the error return code fixes it.

LGTM,

Reviewed-by: Klaus Heinrich Kiwi <klausk@linux.vnet.ibm.com>


> Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>
> ---
>   libstb/drivers/tpm_i2c_nuvoton.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/libstb/drivers/tpm_i2c_nuvoton.c b/libstb/drivers/tpm_i2c_nuvoton.c
> index 3679ddafc..d668000e4 100644
> --- a/libstb/drivers/tpm_i2c_nuvoton.c
> +++ b/libstb/drivers/tpm_i2c_nuvoton.c
> @@ -383,6 +383,7 @@ static int tpm_read_fifo(uint8_t* buf, size_t* buflen)
>   			prlog(PR_ERR, "NUVOTON: overflow on fifo read, c=%zd, "
>   			      "bc=%d, bl=%zd\n", count, burst_count, *buflen);
>   			rc = STB_TPM_OVERFLOW;
> +			goto error;
>   		}
>   		rc = tpm_i2c_request_send(tpm_device,
>   					  SMBUS_READ,
>
Oliver O'Halloran March 30, 2020, 6:26 a.m. UTC | #3
On Fri, Feb 28, 2020 at 4:15 AM Mauro S. M. Rodrigues
<maurosr@linux.vnet.ibm.com> wrote:
>
> The tpm_read_fifo expects buflen parameter which is the size of buf
> parameter. Later it uses buflen to check for an overflow in the case tpm
> response is bigger than buf capacity.
>
> The check is fine, but it doesn't interrupt the code flow, so even though
> we see error messages about the overflow, it doesn't prevent it.
>
> Adding a goto after specifying the error return code fixes it.
>
> Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>

Thanks, merged as e4113f9400ff5366c1bcb395970306612649040b

Patch
diff mbox series

diff --git a/libstb/drivers/tpm_i2c_nuvoton.c b/libstb/drivers/tpm_i2c_nuvoton.c
index 3679ddafc..d668000e4 100644
--- a/libstb/drivers/tpm_i2c_nuvoton.c
+++ b/libstb/drivers/tpm_i2c_nuvoton.c
@@ -383,6 +383,7 @@  static int tpm_read_fifo(uint8_t* buf, size_t* buflen)
 			prlog(PR_ERR, "NUVOTON: overflow on fifo read, c=%zd, "
 			      "bc=%d, bl=%zd\n", count, burst_count, *buflen);
 			rc = STB_TPM_OVERFLOW;
+			goto error;
 		}
 		rc = tpm_i2c_request_send(tpm_device,
 					  SMBUS_READ,