| Message ID | 20200227171505.1793432-1-maurosr@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tpm_i2c_nuvoton: fix tpm_read_fifo overflow check | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (82aed17a5468aff6b600ee1694a10a60f942c018) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
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,
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, >
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
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,
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(+)