linux-kernel - Re: [PATCH v2] tpm_tis: Remove the HID IFX0102

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]

Date:   Mon, 6 Jul 2020 17:43:28 +0300
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     Peter.Huewe@...ineon.com
Cc:     linux-integrity@...r.kernel.org, kjhall@...ibm.com,
        ferry.toth@...inga.info, peterhuewe@....de, jgg@...pe.ca,
        arnd@...db.de, gregkh@...uxfoundation.org, akpm@...l.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] tpm_tis: Remove the HID IFX0102

On Mon, Jul 06, 2020 at 05:00:51PM +0300, Jarkko Sakkinen wrote:
> On Mon, Jul 06, 2020 at 11:46:46AM +0000, Peter.Huewe@...ineon.com wrote:
> > Hi,
> > NACK
> > 
> > > % git --no-pager grep IFX0102 drivers/char/tpm
> > > drivers/char/tpm/tpm_infineon.c:	{"IFX0102", 0},
> > > drivers/char/tpm/tpm_tis.c:	{"IFX0102", 0},		/* Infineon */
> > > Obviously IFX0102 was added to the HID table for the TCG TIS driver by mistake.
> > 
> > The HID IFX0102 was NOT added by mistake.
> > Let me explain the history a bit:
> > 
> > Old SLB 9635 / 9630 TPMs had two ways to interface them
> > - proprietary 'io' mapped protocol (tpm_infineon) - tis protocol  (tpm_tis)
> > 
> > Both match the same HID.
> > However with the emerging of the tis protocol, the io protocol eventually went away for newer products.
> > So all TPM1.2 by IFX match the HID0102 and the TCG generic ones PNP0C31
> > 
> > So basically you break TPM1.2 support for all (newer) Infineon chips if the platform vendor used the IFX0102 HID as they would speak via tpm_infineon driver.
> > The bug must be something different, especially as it only seems to happen after suspend resume.
> 
> Peter,
> 
> Looking at dmesg:
> 
> 1. tmp_infineon initializes cleanly
> 2. tpm_tis fails misserably with bunch error messages
> 
> I'm cool with reverting the patch though. Please send a revert patch and
> explain this in the commit message because right now what you are saying
> is completely undocumented.
> 
> Also, this tpm_infineon issue needs to be fixed properly after the
> revert.
> 
> The bugzilla bug is unrelated to this issue but it causes extra harm
> fixing any bugs and confusion among the users as the bug discussions
> proves.
> 
> How do we get the quirks for tpm_tis and tpm_infineon so that they can
> separate each other?

Also in the revert commit, please add a comment to tpm_tis.c
about the existing conflict, e.g.

/*
 * Legacy Infineon devices can emit illegit warnings as tpm_tis and
 * tpm_infineon have a conflicting device ID IFX0102.
 */

I'm cool reverting it as long as I get a patch with the required
premises to do so and proper documentation, because the issue is
still real.

/Jarkko