On Thu, 2021-11-04 at 16:02 +0200, amirmizi6@xxxxxxxxx wrote:
> From: Amir Mizinski <amirmizi6@xxxxxxxxx>
>
> An invalid TPM_STS value could be used when the following two events occur:
> TPM does not update TPM_STS register after a locality request (TPM_STS
> Initial value = 0xFF), and a TPM_STS register read occurs in the
> tpm_tis_status(chip) function call.
>
> In probe_itpm(), a call to tpm_tis_send_data() function is made after a
> request_locality() call, and the condition
> ("if ((status & TPM_STS_COMMAND_READY) == 0)") is checked. At this moment
> if the status value is 0xFF, then it is considered, wrongly, in “ready”
> state (by checking only one bit). However, at this moment the TPM is, in
> fact, in "Idle" state and remains in "Idle" state because
> "tpm_tis_ready(chip);" was not executed.
> Waiting for the condition TPM_STS.tpmGo == 0, will ensure that the TPM
> status register has the correct value.
You should use imperative form in commit message, e.g. "Wait for TPM_STS.tpmGo
to reset to zero, ...".
>
> Suggested-by: Benoit Houyere <benoit.houyere@xxxxxx>
> Signed-off-by: Amir Mizinski <amirmizi6@xxxxxxxxx>
> ---
> drivers/char/tpm/tpm_tis_core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 6ff8b44..770685a 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -177,8 +177,12 @@ static int request_locality(struct tpm_chip *chip, int l)
> } else {
> /* wait for burstcount */
> do {
> - if (check_locality(chip, l))
> + if (check_locality(chip, l)) {
> + if (tpm_tis_wait_for_stat(chip, TPM_STS_GO, 0, chip->timeout_c,
> + &priv->int_queue, false) < 0)
You would need to explain this with an inline comment.
> + return -ETIME;
> return l;
> + }
> tpm_msleep(TPM_TIMEOUT);
> } while (time_before(jiffies, stop));
> }
/Jarkko
