linux-kernel - Re: [PATCH v2] tpm_tis: Add missing tpm_request/relinquish

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:   Tue, 2 Feb 2021 18:29:19 +0200
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Dirk Gouders <dirk@...ders.net>
Cc:     Lukasz Majczak <lma@...ihalf.com>,
        Guenter Roeck <linux@...ck-us.net>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        Tj <ml.linux@...oe.vision>, Peter Huewe <peterhuewe@....de>,
        Jason Gunthorpe <jgg@...pe.ca>,
        linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        Radoslaw Biernacki <rad@...ihalf.com>,
        Marcin Wojtas <mw@...ihalf.com>,
        Alex Levin <levinale@...gle.com>
Subject: Re: [PATCH v2] tpm_tis: Add missing tpm_request/relinquish_locality
 calls

On Sun, Jan 31, 2021 at 10:43:05AM +0100, Dirk Gouders wrote:
> Jarkko Sakkinen <jarkko@...nel.org> writes:
> 
> > On Thu, 2021-01-28 at 14:07 +0100, Lukasz Majczak wrote:
> >> There is a missing call to tpm_request_locality before the call to
> >> the tpm_get_timeouts() and tpm_tis_probe_irq_single(). As the current
> >> approach might work for tpm2, it fails for tpm1.x - in that case
> >> call to tpm_get_timeouts() or tpm_tis_probe_irq_single()
> >> without locality fails and in turn causes tpm_tis_core_init() to fail.
> >> Tested on Samsung Chromebook Pro (Caroline).
> >> 
> >> Signed-off-by: Lukasz Majczak <lma@...ihalf.com>
> >
> > Is it possible that you test against linux-next and see if any
> > problems still arise? I've applied the locality fixes from James.
> 
> I tested current linux-next and the warning still appears,
> unfortunately.
> 
> I then incrementally applied further patches from James' series [1] and
> after "[PATCH v2 4/5] tpm_tis: fix IRQ probing" the warning has gone:
> 
> # dmesg | grep tpm
> [    7.220410] tpm_tis STM0125:00: 2.0 TPM (device-id 0x0, rev-id 78)
> [    7.322564] Modules linked in: iwlmvm(+) btusb btrtl btbcm btintel mac80211 amdgpu(+) iwlwifi drm_ttm_helper tpm_crb sdhci_pci ttm cqhci gpu_sched sdhci ccp cfg80211 rng_core tpm_tis tpm_tis_core tpm thinkpad_acpi(+) wmi nvram pinctrl_amd
> 
> You might notice there is another warning but that is rtc related and I
> still have to find out if that is something I should report.

1/5 and 2/5 are on the other hand unconditionally "right things" to
do. In your case the resulting state will anyway just result a polling
driver and the kernel state remains stable.

WARN_ONCE() is clearly an overkill, as kernel does not have inconsistent
state. Using WARN_ONCE() was a mistake.

I sent this fix: https://lore.kernel.org/linux-integrity/3936843b-c0da-dd8c-8aa9-90aa3b49d525@linux.ibm.com/T/#t

It will turn WARN_ONCE() to pr_warn_once() and also add the status code
to klog entry. It's sufficient in this case. For 3/5-5/5 we need more
time.

/Jarkko