| 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
| ||
|
Message-ID: <Yr4vUSrShAWntLeP@kernel.org>
Date: Fri, 1 Jul 2022 02:18:41 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: Lino Sanfilippo <LinoSanfilippo@....de>
Cc: peterhuewe@....de, jgg@...pe.ca, stefanb@...ux.vnet.ibm.com,
linux@...ewoehner.de, linux-integrity@...r.kernel.org,
linux-kernel@...r.kernel.org, l.sanfilippo@...bus.com,
lukas@...ner.de, p.rosenberger@...bus.com
Subject: Re: [PATCH v7 05/10] tpm, tpm_tis: Only handle supported interrupts
On Thu, Jun 30, 2022 at 01:26:48AM +0200, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@...bus.com>
>
> According to the TPM Interface Specification (TIS) support for "stsValid"
> and "commandReady" interrupts is only optional.
> This has to be taken into account when handling the interrupts in functions
> like wait_for_tpm_stat(). To determine the supported interrupts use the
> capability query.
>
> Also adjust wait_for_tpm_stat() to only wait for interrupt reported status
> changes. After that process all the remaining status changes by polling
> the status register.
>
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@...bus.com>
> Tested-by: Michael Niew??hner <linux@...ewoehner.de>
> ---
> drivers/char/tpm/tpm_tis_core.c | 119 +++++++++++++++++++-------------
> drivers/char/tpm/tpm_tis_core.h | 1 +
> 2 files changed, 72 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 09d8f04cbc81..c13599e94ab6 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> long rc;
> u8 status;
> bool canceled = false;
> + u8 sts_mask = 0;
> + int ret = 0;
>
> /* check current status */
> status = chip->ops->status(chip);
> if ((status & mask) == mask)
> return 0;
>
> - stop = jiffies + timeout;
> + /* check which status changes can be handled by irqs */
> + if (priv->int_mask & TPM_INTF_STS_VALID_INT)
> + sts_mask |= TPM_STS_VALID;
>
> - if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> + if (priv->int_mask & TPM_INTF_DATA_AVAIL_INT)
> + sts_mask |= TPM_STS_DATA_AVAIL;
> +
> + if (priv->int_mask & TPM_INTF_CMD_READY_INT)
> + sts_mask |= TPM_STS_COMMAND_READY;
> +
> + sts_mask &= mask;
> +
> + stop = jiffies + timeout;
> + /* process status changes with irq support */
> + if (sts_mask) {
> + ret = -ETIME;
> again:
> timeout = stop - jiffies;
> if ((long)timeout <= 0)
> return -ETIME;
> rc = wait_event_interruptible_timeout(*queue,
> - wait_for_tpm_stat_cond(chip, mask, check_cancel,
> + wait_for_tpm_stat_cond(chip, sts_mask, check_cancel,
> &canceled),
> timeout);
> if (rc > 0) {
> if (canceled)
> return -ECANCELED;
> - return 0;
> + ret = 0;
> }
> if (rc == -ERESTARTSYS && freezing(current)) {
> clear_thread_flag(TIF_SIGPENDING);
> goto again;
> }
> - } else {
> - do {
> - usleep_range(priv->timeout_min,
> - priv->timeout_max);
> - status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> - return 0;
> - } while (time_before(jiffies, stop));
> }
> +
> + if (ret)
> + return ret;
> +
> + mask &= ~sts_mask;
> + if (!mask) /* all done */
> + return 0;
> + /* process status changes without irq support */
> + do {
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + usleep_range(priv->timeout_min,
> + priv->timeout_max);
> + } while (time_before(jiffies, stop));
> return -ETIME;
> }
>
> @@ -1007,8 +1029,39 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> if (rc < 0)
> goto out_err;
>
> - intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
> - TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
> + /* Figure out the capabilities */
> + rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
> + if (rc < 0)
> + goto out_err;
> +
> + dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
> + intfcaps);
> + if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
> + dev_dbg(dev, "\tBurst Count Static\n");
> + if (intfcaps & TPM_INTF_CMD_READY_INT) {
> + intmask |= TPM_INTF_CMD_READY_INT;
> + dev_dbg(dev, "\tCommand Ready Int Support\n");
> + }
> + if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
> + dev_dbg(dev, "\tInterrupt Edge Falling\n");
> + if (intfcaps & TPM_INTF_INT_EDGE_RISING)
> + dev_dbg(dev, "\tInterrupt Edge Rising\n");
> + if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
> + dev_dbg(dev, "\tInterrupt Level Low\n");
> + if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
> + dev_dbg(dev, "\tInterrupt Level High\n");
> + if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
> + intmask |= TPM_INTF_LOCALITY_CHANGE_INT;
> + dev_dbg(dev, "\tLocality Change Int Support\n");
> + if (intfcaps & TPM_INTF_STS_VALID_INT) {
> + intmask |= TPM_INTF_STS_VALID_INT;
> + dev_dbg(dev, "\tSts Valid Int Support\n");
> + }
> + if (intfcaps & TPM_INTF_DATA_AVAIL_INT) {
> + intmask |= TPM_INTF_DATA_AVAIL_INT;
> + dev_dbg(dev, "\tData Avail Int Support\n");
> + }
> +
> intmask &= ~TPM_GLOBAL_INT_ENABLE;
>
> rc = request_locality(chip, 0);
> @@ -1042,32 +1095,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> goto out_err;
> }
>
> - /* Figure out the capabilities */
> - rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
> - if (rc < 0)
> - goto out_err;
> -
> - dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
> - intfcaps);
> - if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
> - dev_dbg(dev, "\tBurst Count Static\n");
> - if (intfcaps & TPM_INTF_CMD_READY_INT)
> - dev_dbg(dev, "\tCommand Ready Int Support\n");
> - if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
> - dev_dbg(dev, "\tInterrupt Edge Falling\n");
> - if (intfcaps & TPM_INTF_INT_EDGE_RISING)
> - dev_dbg(dev, "\tInterrupt Edge Rising\n");
> - if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
> - dev_dbg(dev, "\tInterrupt Level Low\n");
> - if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
> - dev_dbg(dev, "\tInterrupt Level High\n");
> - if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
> - dev_dbg(dev, "\tLocality Change Int Support\n");
> - if (intfcaps & TPM_INTF_STS_VALID_INT)
> - dev_dbg(dev, "\tSts Valid Int Support\n");
> - if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
> - dev_dbg(dev, "\tData Avail Int Support\n");
> -
> /* INTERRUPT Setup */
> init_waitqueue_head(&priv->read_queue);
> init_waitqueue_head(&priv->int_queue);
> @@ -1098,7 +1125,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> else
> tpm_tis_probe_irq(chip, intmask);
>
> - if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> + priv->int_mask = intmask;
> + } else {
> dev_err(&chip->dev, FW_BUG
> "TPM interrupt not working, polling instead\n");
>
> @@ -1145,13 +1174,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
> if (rc < 0)
> goto out;
>
> - rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
> - if (rc < 0)
> - goto out;
> -
> - intmask |= TPM_INTF_CMD_READY_INT
> - | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
> - | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
> + intmask = priv->int_mask | TPM_GLOBAL_INT_ENABLE;
>
> tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index bf07379dea42..e005eb99480e 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -93,6 +93,7 @@ struct tpm_tis_data {
> u16 manufacturer_id;
> int locality;
> int irq;
> + unsigned int int_mask;
> unsigned long flags;
> void __iomem *ilb_base_addr;
> u16 clkrun_enabled;
> --
> 2.25.1
>
Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
BR, Jarkko
Powered by blists - more mailing lists