Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Mon, 04 Dec 2023 04:36:13 +0200
Message-Id: <CXF771IUU5TC.2HU7I8Z27K1UF@suppilovahvero>
To:     "Angga" <Hermin.Anggawijaya@alliedtelesis.co.nz>,
        "Stefan Berger" <stefanb@linux.ibm.com>,
        "peterhuewe@gmx.de" <peterhuewe@gmx.de>,
        "jgg@ziepe.ca" <jgg@ziepe.ca>
Cc:     "linux-integrity@vger.kernel.org" <linux-integrity@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tpm: Start the tpm2 before running a self test.
From:   "Jarkko Sakkinen" <jarkko@kernel.org>
References: <20231122065528.1049819-1-hermin.anggawijaya@alliedtelesis.co.nz> <85154bfe-6bd5-440a-acc1-f01497d59af5@linux.ibm.com> <b1d29d38-2d9c-4147-a53d-e240d8a436ae@alliedtelesis.co.nz>
In-Reply-To: <b1d29d38-2d9c-4147-a53d-e240d8a436ae@alliedtelesis.co.nz>
Precedence: bulk

On Mon Nov 27, 2023 at 4:02 AM EET, Angga wrote:
> On 23/11/2023 1:34 am, Stefan Berger wrote:
> >
> >
> > On 11/22/23 01:55, Hermin Anggawijaya wrote:
> >> Before sending a command to attempt the self test, the TPM
> >> may need to be started, otherwise the self test returns
> >> TPM2_RC_INITIALIZE value causing a log as follows:
> >> "tpm tpm0: A TPM error (256) occurred attempting the self test".
> >>
> >> Signed-off-by: Hermin Anggawijaya=20
> >> <hermin.anggawijaya@alliedtelesis.co.nz>
> >> ---
> >> =C2=A0 drivers/char/tpm/tpm2-cmd.c | 8 ++++----
> >> =C2=A0 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> >> index 93545be190a5..0530f3b5f86a 100644
> >> --- a/drivers/char/tpm/tpm2-cmd.c
> >> +++ b/drivers/char/tpm/tpm2-cmd.c
> >> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (rc)
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto out;
> >> =C2=A0 +=C2=A0=C2=A0=C2=A0 rc =3D tpm2_startup(chip);
> >> +=C2=A0=C2=A0=C2=A0 if (rc && rc !=3D TPM2_RC_INITIALIZE)
> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto out;
> >> +
> >
> > Most platforms should have firmware initialize the TPM 2 these days.=20
> > Therefore, a selftest should work and in case it doesn't work you fall=
=20
> > back to the tpm2_startup below and if you get an error message in the=
=20
> > log you at least know that you firmware is not up-to-date.
> >
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rc =3D tpm2_do_selftest(chip);
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (rc && rc !=3D TPM2_RC_INITIALIZE)
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto out;
> >> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (rc =3D=3D TPM2_RC_INITIALIZE=
) {
> >> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rc =3D tpm2_startup(chip);
> >> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (rc)
> >> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 go=
to out;
> >> -
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rc =3D tpm2_do_=
selftest(chip);
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (rc)
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0 goto out;
>
> Hello Stefan
>
> Thank you for your comments.
>
> Unfortunately our platforms (custom hardware design) are the ones which=
=20
> do not initialize/start the TPM2 from boot loader yet, and because of=20
> that the
> self test in tpm2_auto_startup always produce a log error message on the=
=20
> platform start up.
>
> While I understand your point about the log being useful for "pointing=20
> out not up-to-date firmware", but it might also generate unnecessary supp=
ort
> queries from some users on such platforms ? And maybe the kernel being=20
> able to deal with TPM being started more than once is better ?
>
> If wanted, I have the second version of the patch which consist of code=
=20
> changes as in v1, plus ability for tpm2_transmit_cmd to handle multiple
> attempts to start up the TPM silently, for example, once by the firmware=
=20
> and another by the kernel during tpm2 auto-startup.

To save your time: no.

Mainline kernel is not modified based hardware prototypes.

You have freedom to maintain your own kernel tree for whatever changes
you need but this is totally wrong place for these type of patches.

>
> Kind regards
>
> Hermin Anggawijaya

BR, Jarkko