From: Sergey Temerkhanov <s.temerkhanov@gmail.com>
To: Peter Huewe <peterhuewe@gmx.de>,
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
Jason Gunthorpe <jgg@ziepe.ca>, Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Sergey Temerkhanov <s.temerkhanov@gmail.com>,
Jerry Snitselaar <jsnitsel@redhat.com>,
linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH][RFC] tpm: Rework open/close/shutdown to avoid races
Date: Fri, 4 Dec 2020 13:18:05 +0300 [thread overview]
Message-ID: <20201204101805.27374-1-s.temerkhanov@gmail.com> (raw)
Avoid race condition at shutdown by shutting downn the TPM 2.0
devices synchronously. This eliminates the condition when the
shutdown sequence sets chip->ops to NULL leading to the following:
[ 1586.593561][ T8669] tpm2_del_space+0x28/0x73
[ 1586.598718][ T8669] tpmrm_release+0x27/0x33wq
[ 1586.603774][ T8669] __fput+0x109/0x1d
[ 1586.608380][ T8669] task_work_run+0x7c/0x90
[ 1586.613414][ T8669] prepare_exit_to_usermode+0xb8/0x128
[ 1586.619522][ T8669] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1586.626068][ T8669] RIP: 0033:0x4cb4bb
Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
---
drivers/char/tpm/tpm-chip.c | 27 ---------------------------
drivers/char/tpm/tpm-dev.c | 11 ++++++-----
drivers/char/tpm/tpmrm-dev.c | 7 +++++++
include/linux/tpm.h | 2 +-
4 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 1838039b0333..ede7f4790c5e 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -282,32 +282,6 @@ static void tpm_devs_release(struct device *dev)
put_device(&chip->dev);
}
-/**
- * tpm_class_shutdown() - prepare the TPM device for loss of power.
- * @dev: device to which the chip is associated.
- *
- * Issues a TPM2_Shutdown command prior to loss of power, as required by the
- * TPM 2.0 spec. Then, calls bus- and device- specific shutdown code.
- *
- * Return: always 0 (i.e. success)
- */
-static int tpm_class_shutdown(struct device *dev)
-{
- struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
-
- down_write(&chip->ops_sem);
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
- if (!tpm_chip_start(chip)) {
- tpm2_shutdown(chip, TPM2_SU_CLEAR);
- tpm_chip_stop(chip);
- }
- }
- chip->ops = NULL;
- up_write(&chip->ops_sem);
-
- return 0;
-}
-
/**
* tpm_chip_alloc() - allocate a new struct tpm_chip instance
* @pdev: device to which the chip is associated
@@ -347,7 +321,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
device_initialize(&chip->devs);
chip->dev.class = tpm_class;
- chip->dev.class->shutdown_pre = tpm_class_shutdown;
chip->dev.release = tpm_dev_release;
chip->dev.parent = pdev;
chip->dev.groups = chip->groups;
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index e2c0baa69fef..e04f3d47c64e 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -23,9 +23,9 @@ static int tpm_open(struct inode *inode, struct file *file)
chip = container_of(inode->i_cdev, struct tpm_chip, cdev);
/* It's assured that the chip will be opened just once,
- * by the check of is_open variable, which is protected
- * by driver_lock. */
- if (test_and_set_bit(0, &chip->is_open)) {
+ * by the check of is_open variable
+ */
+ if (atomic_fetch_or(1, &chip->is_open)) {
dev_dbg(&chip->dev, "Another process owns this TPM\n");
return -EBUSY;
}
@@ -39,7 +39,7 @@ static int tpm_open(struct inode *inode, struct file *file)
return 0;
out:
- clear_bit(0, &chip->is_open);
+ atomic_set(&chip->is_open, 0);
return -ENOMEM;
}
@@ -49,9 +49,10 @@ static int tpm_open(struct inode *inode, struct file *file)
static int tpm_release(struct inode *inode, struct file *file)
{
struct file_priv *priv = file->private_data;
+ struct tpm_chip *chip = priv->chip;
tpm_common_release(file, priv);
- clear_bit(0, &priv->chip->is_open);
+ atomic_set(&chip->is_open, 0);
kfree(priv);
return 0;
diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
index eef0fb06ea83..ec83ca8105b8 100644
--- a/drivers/char/tpm/tpmrm-dev.c
+++ b/drivers/char/tpm/tpmrm-dev.c
@@ -27,6 +27,8 @@ static int tpmrm_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
+ atomic_inc(&chip->is_open);
+
tpm_common_open(file, chip, &priv->priv, &priv->space);
return 0;
@@ -39,6 +41,11 @@ static int tpmrm_release(struct inode *inode, struct file *file)
tpm_common_release(file, fpriv);
tpm2_del_space(fpriv->chip, &priv->space);
+
+ if (!atomic_dec_return(&fpriv->chip->is_open)) {
+ tpm2_shutdown(fpriv->chip, TPM2_SU_CLEAR);
+ tpm_chip_stop(fpriv->chip);
+ }
kfree(priv);
return 0;
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 77fdc988c610..26e070198a15 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -126,7 +126,7 @@ struct tpm_chip {
unsigned int flags;
int dev_num; /* /dev/tpm# */
- unsigned long is_open; /* only one allowed */
+ atomic_t is_open; /* only one allowed */
char hwrng_name[64];
struct hwrng hwrng;
--
2.25.1
next reply other threads:[~2020-12-04 10:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-04 10:18 Sergey Temerkhanov [this message]
2020-12-04 12:58 ` Jason Gunthorpe
2020-12-08 11:00 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201204101805.27374-1-s.temerkhanov@gmail.com \
--to=s.temerkhanov@gmail.com \
--cc=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=jgg@ziepe.ca \
--cc=jsnitsel@redhat.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterhuewe@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®