linux-kernel - Re: [PATCH] tpm: Add flag to use default cancellation policy

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] [day] [month] [year] [list]

Message-ID: <Yzdkdj7Ma3/RnCCs@kernel.org>
Date:   Sat, 1 Oct 2022 00:49:42 +0300
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Joel Stanley <joel@....id.au>
Cc:     Eddie James <eajames@...ux.ibm.com>,
        linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        peterhuewe@....de, jarkko@...nel.or, jgg@...pe.ca,
        Alexander.Steffen@...ineon.com
Subject: Re: [PATCH] tpm: Add flag to use default cancellation policy

On Wed, Sep 28, 2022 at 05:10:25AM +0000, Joel Stanley wrote:
> On Thu, 8 Sept 2022 at 13:53, Eddie James <eajames@...ux.ibm.com> wrote:
> >
> >
> > On 9/8/22 00:22, Jarkko Sakkinen wrote:
> > > On Wed, Sep 07, 2022 at 11:43:17AM -0500, Eddie James wrote:
> > >> The check for cancelled request depends on the VID of the chip, but
> > >> some chips share VID which shouldn't share their cancellation
> > >> behavior. This is the case for the Nuvoton NPCT75X, which should use
> > >> the default cancellation check, not the Winbond one.
> > >> To avoid changing the existing behavior, add a new flag to indicate
> > >> that the chip should use the default cancellation check and set it
> > >> for the I2C TPM2 TIS driver.
> > >>
> > >> Signed-off-by: Eddie James <eajames@...ux.ibm.com>
> > >> ---
> > >>   drivers/char/tpm/tpm_tis_core.c | 18 ++++++++++--------
> > >>   drivers/char/tpm/tpm_tis_core.h |  1 +
> > >>   drivers/char/tpm/tpm_tis_i2c.c  |  1 +
> > >>   3 files changed, 12 insertions(+), 8 deletions(-)
> > >>
> > >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> > >> index 757623bacfd5..175e75337395 100644
> > >> --- a/drivers/char/tpm/tpm_tis_core.c
> > >> +++ b/drivers/char/tpm/tpm_tis_core.c
> > >> @@ -682,15 +682,17 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
> > >>   {
> > >>      struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> > >>
> > >> -    switch (priv->manufacturer_id) {
> > >> -    case TPM_VID_WINBOND:
> > >> -            return ((status == TPM_STS_VALID) ||
> > >> -                    (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
> > >> -    case TPM_VID_STM:
> > >> -            return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
> > >> -    default:
> > >> -            return (status == TPM_STS_COMMAND_READY);
> > >> +    if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
> > >> +            switch (priv->manufacturer_id) {
> > >> +            case TPM_VID_WINBOND:
> > >> +                    return ((status == TPM_STS_VALID) ||
> > >> +                            (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
> > >> +            case TPM_VID_STM:
> > >> +                    return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
> > >> +            }
> > > Why there is no default: ?
> >
> >
> > Well I didn't want to duplicate the line "status ==
> > TPM_STS_COMMAND_READY" in the default case and for the flagged case. So
> > now the switch just falls through for default. I can add default: break
> > instead
> 
> This code was in the original patch series submitted by Nuvoton:
> 
> https://lore.kernel.org/r/20211104140211.6258-3-amirmizi6@gmail.com
> 
> Perhaps something like that would be better?

The current patch could have 

        default:
                /* fall-through */
                break;

BR, Jarkko