linux-kernel - Re: [PATCH v19 2/5] tpm: tpm_tis: Rewrite "tpm_tis_req

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: <640dd9a5bdb4c28b7429a6ab9507d645de06c125.camel@kernel.org>
Date:   Wed, 17 Nov 2021 10:03:58 +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 2/5] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()"

On Thu, 2021-11-04 at 16:02 +0200, amirmizi6@...il.com wrote:
> From: Amir Mizinski <amirmizi6@...il.com>
> 
> tpm_tis_req_canceled() function is used to check if the caller requested
> to abort the current operation. It was found that in some cases
> tpm_tis_req_canceled() wrongly returned true.

Please, bring some context, i.e. please describe the scenarios.

> Since a cancel request sets the TPM_STS.commandReady field to TRUE, the 
          ~~~~~~~~~~~~~~

What the heck is this? Please refer the exact things.


> tpm_tis_req_canceled() function should check only the TPM_STS.commandReady
> field value.

Why?

> The case for TPM_VID_WINBOND is wrong and was therefore removed.

Why?

It is not removed in the existing mainline, so it definitely *was not*
removed.

> Also, the default comparison is wrong. Only cmdReady bit needs to be
> compared instead of the full lower status register byte.

You should split this into two patches, if it fixes two different
issues.

/Jarkko