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
| ||
|
Message-ID: <cdeec193f1c1f5294581f349253e8863e64c9f5b.camel@kernel.org> Date: Tue, 26 Oct 2021 02:41:12 +0300 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 v17 1/6] tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception On Sun, 2021-10-24 at 19:48 +0300, amirmizi6@...il.com wrote: > From: Amir Mizinski <amirmizi6@...il.com> > > Currently, the driver polls the TPM_STS.stsValid field until TRUE; then it > reads TPM_STS register again to verify only that TPM_STS.expect field is > FALSE (i.e., it ignores TPM_STS.stsValid). > Since TPM_STS.stsValid represents the TPM_STS.expect validity, a check of > only one of these fields is wrong. Fix this condition so that both fields > are checked in the same TPM_STS register read. > > Modify the signature of wait_for_tpm_stat(), adding an additional > "result" parameter to its call. > wait_for_tpm_stat() is now polling the TPM_STS with a mask and waits > for the value in result. This modification adds the ability to check if > certain TPM_STS bits have been cleared. > For example, use the new parameter to check in status that TPM_STS_VALID > is set and also that TPM_STS_EXPECT is zeroed. This prevents a racy > check. > > Fixes: 27084efee0c3 ("tpm: driver for next generation TPM chips") Where does this failure occur in practice? If nowhere, this can be dropped, and the patch can be considered as a feature. Defining it as a fix makes only sense, if it needs to be backported to stable kernels. This requires something to be actually broken. The commit message does not contain a real bug report. It is just referring to the specification, which is not a workload. > Suggested-by: Benoit Houyere <benoit.houyere@...com> > Signed-off-by: Amir Mizinski <amirmizi6@...il.com> > --- > drivers/char/tpm/tpm_tis_core.c | 59 ++++++++++++++++++----------------------- > 1 file changed, 26 insertions(+), 33 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 69579ef..98de2fd 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -44,9 +44,9 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask, > return false; > } > > -static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, > - unsigned long timeout, wait_queue_head_t *queue, > - bool check_cancel) > +static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, u8 result, > + unsigned long timeout, > + wait_queue_head_t *queue, bool check_cancel) I would consider renaming this as, given that you are changing the signature anyway: tpm_tis_wait_for_stat() This would be more consistent with the other naming, and make e.g. grepping kernel tree easier. How did you end up to the name "result"? I have hard time deriving from that name the actual semantics. E.g. "expected" would already a way more sane name. /Jarkko
Powered by blists - more mailing lists