linux-kernel - Re: [PATCH v19 3/5] tpm: tpm_tis: Verify TPM_STS register is valid after locality request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fd914758628f416db9fbaa35e910cceedabb65f7.camel@kernel.org>
Date:   Wed, 17 Nov 2021 10:10:20 +0200
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     amirmizi6@...il.com, Eyal.Cohen@...oton.com,
        oshrialkoby85@...il.com, alexander.steffen@...ineon.com,
        robh+dt@...nel.org, mark.rutland@....com, peterhuewe@....de,
        jgg@...pe.ca, arnd@...db.de, gregkh@...uxfoundation.org,
        benoit.houyere@...com, eajames@...ux.ibm.com, joel@....id.au
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-integrity@...r.kernel.org, oshri.alkoby@...oton.com,
        tmaimon77@...il.com, gcwilson@...ibm.com, kgoldman@...ibm.com,
        Dan.Morav@...oton.com, oren.tanami@...oton.com,
        shmulik.hager@...oton.com, amir.mizinski@...oton.com
Subject: Re: [PATCH v19 3/5] tpm: tpm_tis: Verify TPM_STS register is valid
 after locality request

On Thu, 2021-11-04 at 16:02 +0200, amirmizi6@...il.com wrote:
> From: Amir Mizinski <amirmizi6@...il.com>
> 
> 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@...com>
> Signed-off-by: Amir Mizinski <amirmizi6@...il.com>
> ---
>  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