From patchwork Wed Jun 29 23:26:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lino Sanfilippo X-Patchwork-Id: 12900796 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EDC4CCA47E for ; Wed, 29 Jun 2022 23:27:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231378AbiF2X1k (ORCPT ); Wed, 29 Jun 2022 19:27:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33324 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231159AbiF2X1i (ORCPT ); Wed, 29 Jun 2022 19:27:38 -0400 Received: from mout.gmx.net (mout.gmx.net [212.227.15.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1B48225586; Wed, 29 Jun 2022 16:27:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1656545243; bh=DIND2TdGrcdcYcqNbcqvgd28SywcZMNNgfngQqDPZoI=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date:In-Reply-To:References; b=WEN0wniqlUYqgtosjaxt1AVqRjrbpV+PQrTchivii79X+O4e/VnFlhBfggY9l5uV8 HAUyH2ji7Zu3lZPjvlmt/X/japjdHbwnLg4zAjYfAEYlDSwx6Aa6DFj825KL+x2ozK g76JGIYvWoVYmz9RIX9yAserdPnJ14b3EP82/C2k= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from localhost.localdomain ([46.223.3.23]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1N3bWr-1ngwe02cgl-010ggW; Thu, 30 Jun 2022 01:27:23 +0200 From: Lino Sanfilippo To: peterhuewe@gmx.de, jarkko@kernel.org, jgg@ziepe.ca Cc: stefanb@linux.vnet.ibm.com, linux@mniewoehner.de, linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org, l.sanfilippo@kunbus.com, LinoSanfilippo@gmx.de, lukas@wunner.de, p.rosenberger@kunbus.com Subject: [PATCH v7 01/10] tpm, tpm_tis: Avoid cache incoherency in test for interrupts Date: Thu, 30 Jun 2022 01:26:44 +0200 Message-Id: <20220629232653.1306735-2-LinoSanfilippo@gmx.de> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220629232653.1306735-1-LinoSanfilippo@gmx.de> References: <20220629232653.1306735-1-LinoSanfilippo@gmx.de> MIME-Version: 1.0 X-Provags-ID: V03:K1:+VDe8/cKLoCmkTb+aHJfyXV9TreCCa6tzO/Fe8CFo/bXIy7RqZ8 AgH5CgNC3PX7/LALbPcvwACkjiz0b6vjXQ5fCzTXsmEzkoT3JvSbY/lqsuS35gDXbwClC5b d3UfRudOBi/4THwQBCVSoOEKyoLJp6CY/h53lsr5hCtxgMpRcxUYgdnVQa/0JMAQOdMd1/8 bbjcp6vz/FulqEC2+cVhQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:GvRzEbue8FE=:kMLBGcBRiAZcH1k2Dhyb4L WfZ/Ngo6b485xHUNB69L3blM/bYOLDS5Tq6MhvVJ0ZOmbZJC9vlsI9lKo0KCK9MwYVb4rqUtl 5xHB6hVGQN30ss2WOPVSdxijaI1w3hhBl16MgU6pP+Omc1t8wCwz/ZY8hfXbMXeNIeypgTgOQ EiRIhOxBBhPls/1aSAO1EGcD3m61YtMaPMWzTKUUdsDaBUDfdxLVpQTyYLJUgmNYMtO9P1r1n lCm+GG7coQp6JvUEScq7NHgnM3sQL7RLT3pzaXJWTjZho8dvkLUid5Cb3Ovibl/DiDfYUJR9y +PxkVEtmTez7nWjbdguoNq1635EvPMeBP4QnAMfA0Ws9YrZLV/ZUMLp38uYi/NhwaDMNfjmIt Mz+JE8lFiJs06rGhn2Nmo173KTh5PQXPRBNo3q93SrBbL4TFoRhKbsWNrjpo3otoQ3xgjIcI1 LFri5FpCFxlBpT+MaWvww/QQC8no/kSuxobQxV7IpUS2YfjHmE0WqrY69MJLT/8OBwRXnFs2X 2V7JQJ97HtcV4iXjxkCkFWU3E8qGnWwPSf/QluZk2DkZI2ey5CDia4qKKptOjkSLhbajDVnzZ Y32ILTr8hL+LHVETtVbjK7P2cwnzDoJ1dL2NkAxjE9deTLPbQnAFuAjDN/DVmeK3luamPKJhc WDJyN/TTZcq/E7IL56LnRrjDiD8rYbYyVZ+Og2d76nJW+j9Y7HjQ9Ck6xoNtq6py34bAUBioK FUogdd4JtoaQBdMppzuBHAxDXYc8Acq/tLPh1y+Ohp4Fup7JUa687IDhXwVMha+rBx1I3+E3u IJBMx16pSxC5ptzCUhLSeF0oVOhOjW9pMujDki1c0XeQfLlkYpM+E8bHMOPe3YpxTTbTplKXg B9w7eBVvHeWOWxuCfdjkii2SVNUwGJyohG38fe2/pWGoMLk/EwR9SUHMidNimkLLmEcuVMX4v dgqIigJPHox/A1EVxXHxGEgUbRhnlO0kMRUqMXJnPOS3XqsQlGXID1uDXSrSaZIh4Dggumt4k 4myt4ZE5/nWL0AdZLPbdmfS2n2sHphEqXiIm0kHjrSH09KsaZ6sgJ7//PU5p4r58oEMbygLuu xEsYUAwB2CO87b7e8LdwYCyDm18P6XLnnsCGJcnShga/1XlDX9c8ZHq4Q== Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org From: Lino Sanfilippo The interrupt handler that sets the boolean variable irq_tested may run on another CPU as the thread that checks irq_tested as part of the irq test in tmp_tis_send(). Since nothing guarantees cache coherency between CPUs for unsynchronized accesses to boolean variables the testing thread might not perceive the value change done in the interrupt handler. Avoid this issue by setting the bit TPM_TIS_IRQ_TESTED in the flags field of the tpm_tis_data struct and by accessing this field with the bit manipulating functions that provide cache coherency. Also convert all other existing sites to use the proper macros when accessing this bitfield. Signed-off-by: Lino Sanfilippo Reviewed-by: Jarkko Sakkinen Tested-by: Michael Niewöhner --- drivers/char/tpm/tpm_tis.c | 2 +- drivers/char/tpm/tpm_tis_core.c | 21 +++++++++++---------- drivers/char/tpm/tpm_tis_core.h | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index bcff6429e0b4..ce43412eb398 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -226,7 +226,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) irq = tpm_info->irq; if (itpm || is_itpm(ACPI_COMPANION(dev))) - phy->priv.flags |= TPM_TIS_ITPM_WORKAROUND; + set_bit(TPM_TIS_ITPM_WORKAROUND, &phy->priv.flags); return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg, ACPI_HANDLE(dev)); diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index dc56b976d816..b5fd4ff46666 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -343,7 +343,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); int rc, status, burstcnt; size_t count = 0; - bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; + bool itpm = test_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags); status = tpm_tis_status(chip); if ((status & TPM_STS_COMMAND_READY) == 0) { @@ -470,7 +470,8 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) int rc, irq; struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); - if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested) + if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || + test_bit(TPM_TIS_IRQ_TESTED, &priv->flags)) return tpm_tis_send_main(chip, buf, len); /* Verify receipt of the expected IRQ */ @@ -480,11 +481,11 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) rc = tpm_tis_send_main(chip, buf, len); priv->irq = irq; chip->flags |= TPM_CHIP_FLAG_IRQ; - if (!priv->irq_tested) + if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->flags)) tpm_msleep(1); - if (!priv->irq_tested) + if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->flags)) disable_interrupts(chip); - priv->irq_tested = true; + set_bit(TPM_TIS_IRQ_TESTED, &priv->flags); return rc; } @@ -627,7 +628,7 @@ static int probe_itpm(struct tpm_chip *chip) size_t len = sizeof(cmd_getticks); u16 vendor; - if (priv->flags & TPM_TIS_ITPM_WORKAROUND) + if (test_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags)) return 0; rc = tpm_tis_read16(priv, TPM_DID_VID(0), &vendor); @@ -647,13 +648,13 @@ static int probe_itpm(struct tpm_chip *chip) tpm_tis_ready(chip); - priv->flags |= TPM_TIS_ITPM_WORKAROUND; + set_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags); rc = tpm_tis_send_data(chip, cmd_getticks, len); if (rc == 0) dev_info(&chip->dev, "Detected an iTPM.\n"); else { - priv->flags &= ~TPM_TIS_ITPM_WORKAROUND; + clear_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags); rc = -EFAULT; } @@ -693,7 +694,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id) if (interrupt == 0) return IRQ_NONE; - priv->irq_tested = true; + set_bit(TPM_TIS_IRQ_TESTED, &priv->flags); if (interrupt & TPM_INTF_DATA_AVAIL_INT) wake_up_interruptible(&priv->read_queue); if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT) @@ -779,7 +780,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask, if (rc < 0) return rc; - priv->irq_tested = false; + clear_bit(TPM_TIS_IRQ_TESTED, &priv->flags); /* Generate an interrupt by having the core call through to * tpm_tis_send diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h index 6c203f36b8a1..bf07379dea42 100644 --- a/drivers/char/tpm/tpm_tis_core.h +++ b/drivers/char/tpm/tpm_tis_core.h @@ -86,13 +86,13 @@ enum tis_defaults { enum tpm_tis_flags { TPM_TIS_ITPM_WORKAROUND = BIT(0), TPM_TIS_INVALID_STATUS = BIT(1), + TPM_TIS_IRQ_TESTED = BIT(2), }; struct tpm_tis_data { u16 manufacturer_id; int locality; int irq; - bool irq_tested; unsigned long flags; void __iomem *ilb_base_addr; u16 clkrun_enabled;