linux-kernel - Wrong/strange TPM patches was Re: [PATCH 6.1 000/119] 6.1.31-rc1 review

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: <ZHXUGdFMIcB5HL8s@duo.ucw.cz>
Date:   Tue, 30 May 2023 12:46:49 +0200
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     stable@...r.kernel.org, patches@...ts.linux.dev,
        linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
        akpm@...ux-foundation.org, linux@...ck-us.net, shuah@...nel.org,
        patches@...nelci.org, lkft-triage@...ts.linaro.org, pavel@...x.de,
        jonathanh@...dia.com, f.fainelli@...il.com,
        sudipm.mukherjee@...il.com, srw@...dewatkins.net, rwarsow@....de,
        l.sanfilippo@...bus.com, jarkko@...nel.org
Subject: Wrong/strange TPM patches was Re: [PATCH 6.1 000/119] 6.1.31-rc1
 review

Hi!

> This is the start of the stable review cycle for the 6.1.31 release.
> There are 119 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

> Lino Sanfilippo <l.sanfilippo@...bus.com>
>     tpm, tpm_tis: Avoid cache incoherency in test for interrupts

Description on this one is wrong/confused. There's no cache problem in
the code. Plus test_bit and friend already use bit number, so

-       bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
+       bool itpm = test_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags);

@@ -87,6 +87,7 @@ enum tpm_tis_flags {
        TPM_TIS_ITPM_WORKAROUND         = BIT(0),
        TPM_TIS_INVALID_STATUS          = BIT(1),
        TPM_TIS_DEFAULT_CANCELLATION    = BIT(2),
+       TPM_TIS_IRQ_TESTED              = BIT(3),
 };

this enum needs to go from BIT() to raw numbers.

You can just do return tpm_pm_resume();

> Jarkko Sakkinen <jarkko@...nel.org>
>     tpm: Prevent hwrng from activating during resume

@@ -429,6 +431,14 @@ int tpm_pm_resume(struct device *dev)
        if (chip == NULL)
                return -ENODEV;
 
+       chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
+
+       /*
+        * Guarantee that SUSPENDED is written last, so that hwrng does not
+        * activate before the chip has been fully resumed.
+        */
+       wmb();
+
        return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_pm_resume);

This code is confused. First, either you don't need memory barriers
here, or you need real locking. Second, if you want to guarantee flags
are written last, you need to put the barrier before the
assignment. (But ... get rid of that confusion, first).

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)