On Fri, Oct 21, 2016 at 9:13 AM, Jarkko Sakkinen
<jarkko.sakki...@linux.intel.com> wrote:
> On Thu, Oct 20, 2016 at 05:21:29PM -0700, Josh Zimmerman wrote:
>> If the TPM we're connecting to uses a static burst count, it will report
>> a burst count of zero throughout the response read. However, get_burstcount
>> assumes that a response of zero indicates that the TPM is not ready to
>> receive more data. In this case, it returns a negative error code, which
>> is passed on to tpm_tis_{write,read}_bytes as a u16, causing
>> them to read/write far too many bytes.
>>
>> This patch checks for negative return codes and bails out from recv_data
>> and tpm_tis_send_data.
>
> I guess this would need a "Fixes:" tag, wouldn't it?
Ah, yes. Good point.

>  I would also add
>
> Cc: sta...@vger.kernel.org
Will do when I mail the updated patch.

>> ---
>>  drivers/char/tpm/tpm_tis_core.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c 
>> b/drivers/char/tpm/tpm_tis_core.c
>> index e3bf31b..d0301dc 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -186,6 +186,12 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, 
>> size_t count)
>>                                chip->timeout_c,
>>                                &priv->read_queue, true) == 0) {
>>               burstcnt = min_t(int, get_burstcount(chip), count - size);
>> +             if (burstcnt < 0) {
>> +                     dev_err(&chip->dev,
>> +                             "Unable to read burstcount in %s:%d (%s)\n",
>> +                             __FILE__, __LINE__, __func__);
>> +                     return rc;
>> +             }
>
> "return burstcnt;"
Done, thanks!

> I guess __func__ would be enough to deduce the call site?
It is not; would you suggest removing it since it's redundant with
__LINE__ and __FILE__? Or is there some other macro you know of that
could be useful for determining the call site? (I had included it
initially for the sake of ease of log reading, but that's perhaps not
very compelling.)

>
>>               rc = tpm_tis_read_bytes(priv, TPM_DATA_FIFO(priv->locality),
>>                                       burstcnt, buf + size);
>> @@ -272,6 +278,13 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 
>> *buf, size_t len)
>>
>>       while (count < len - 1) {
>>               burstcnt = min_t(int, get_burstcount(chip), len - count - 1);
>> +             if (burstcnt < 0) {
>> +                     dev_err(&chip->dev,
>> +                             "Unable to read burstcount in %s:%d (%s)\n",
>> +                             __FILE__, __LINE__, __func__);
>> +                     rc = burstcnt;
>> +                     goto out_err;
>> +             }
>>               rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>>                                        burstcnt, buf + count);
>>               if (rc < 0)
>> --
>> 2.8.0.rc3.226.g39d4020
>
> /Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Reply via email to