[PATCH v2] tpm: Add explicit chip->ops locking for sysfs attributes.
From: Guenter Roeck
Date: Thu Nov 16 2017 - 16:41:12 EST
Add explicit chip->ops locking for all sysfs attributes.
This lets us support those attributes on tpm2 devices.
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
v2: It makes sense to actually compile the code.
drivers/char/tpm/tpm-chip.c | 4 --
drivers/char/tpm/tpm-sysfs.c | 127 ++++++++++++++++++++++++++++++++-----------
2 files changed, 94 insertions(+), 37 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 0eca20c5a80c..f0593ec1c11b 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -149,10 +149,6 @@ static void tpm_devs_release(struct device *dev)
* 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.
- *
- * XXX: This codepath relies on the fact that sysfs is not enabled for
- * TPM2: sysfs uses an implicit lock on chip->ops, so this could race if TPM2
- * has sysfs support enabled before TPM sysfs's implicit locking is fixed.
*/
static int tpm_class_shutdown(struct device *dev)
{
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 83a77a445538..13405bdde725 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -47,18 +47,22 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
memset(&anti_replay, 0, sizeof(anti_replay));
- rc = tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK);
+ rc = tpm_try_get_ops(chip);
if (rc)
return rc;
+ rc = tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK);
+ if (rc)
+ goto put_ops;
+
tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
rc = tpm_transmit_cmd(chip, NULL, tpm_buf.data, PAGE_SIZE,
READ_PUBEK_RESULT_MIN_BODY_SIZE, 0,
"attempting to read the PUBEK");
if (rc) {
- tpm_buf_destroy(&tpm_buf);
- return 0;
+ rc = 0;
+ goto buf_destroy;
}
out = (struct tpm_readpubek_out *)&tpm_buf.data[10];
@@ -91,7 +95,10 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
}
rc = str - buf;
+buf_destroy:
tpm_buf_destroy(&tpm_buf);
+put_ops:
+ tpm_put_ops(chip);
return rc;
}
static DEVICE_ATTR_RO(pubek);
@@ -106,11 +113,17 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
char *str = buf;
struct tpm_chip *chip = to_tpm_chip(dev);
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
"attempting to determine the number of PCRS",
sizeof(cap.num_pcrs));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
num_pcrs = be32_to_cpu(cap.num_pcrs);
for (i = 0; i < num_pcrs; i++) {
@@ -122,23 +135,35 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
str += sprintf(str, "%02X ", digest[j]);
str += sprintf(str, "\n");
}
- return str - buf;
+ rc = str - buf;
+put_ops:
+ tpm_put_ops(chip);
+ return rc;
}
static DEVICE_ATTR_RO(pcrs);
static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct tpm_chip *chip = to_tpm_chip(dev);
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm_getcap(chip, TPM_CAP_FLAG_PERM, &cap,
"attempting to determine the permanent enabled state",
sizeof(cap.perm_flags));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
rc = sprintf(buf, "%d\n", !cap.perm_flags.disable);
+put_ops:
+ tpm_put_ops(chip);
return rc;
}
static DEVICE_ATTR_RO(enabled);
@@ -146,16 +171,25 @@ static DEVICE_ATTR_RO(enabled);
static ssize_t active_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct tpm_chip *chip = to_tpm_chip(dev);
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm_getcap(chip, TPM_CAP_FLAG_PERM, &cap,
"attempting to determine the permanent active state",
sizeof(cap.perm_flags));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
rc = sprintf(buf, "%d\n", !cap.perm_flags.deactivated);
+put_ops:
+ tpm_put_ops(chip);
return rc;
}
static DEVICE_ATTR_RO(active);
@@ -163,16 +197,25 @@ static DEVICE_ATTR_RO(active);
static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct tpm_chip *chip = to_tpm_chip(dev);
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm_getcap(chip, TPM_CAP_PROP_OWNER, &cap,
"attempting to determine the owner state",
sizeof(cap.owned));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
rc = sprintf(buf, "%d\n", cap.owned);
+put_ops:
+ tpm_put_ops(chip);
return rc;
}
static DEVICE_ATTR_RO(owned);
@@ -180,16 +223,25 @@ static DEVICE_ATTR_RO(owned);
static ssize_t temp_deactivated_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
+ struct tpm_chip *chip = to_tpm_chip(dev);
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm_getcap(chip, TPM_CAP_FLAG_VOL, &cap,
"attempting to determine the temporary state",
sizeof(cap.stclear_flags));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
rc = sprintf(buf, "%d\n", cap.stclear_flags.deactivated);
+put_ops:
+ tpm_put_ops(chip);
return rc;
}
static DEVICE_ATTR_RO(temp_deactivated);
@@ -202,11 +254,18 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
ssize_t rc;
char *str = buf;
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
"attempting to determine the manufacturer",
sizeof(cap.manufacturer_id));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
+
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
@@ -226,8 +285,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
rc = tpm_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
"attempting to determine the 1.1 version",
sizeof(cap.tpm_version));
- if (rc)
- return 0;
+ if (rc) {
+ rc = 0;
+ goto put_ops;
+ }
str += sprintf(str,
"TCG version: %d.%d\nFirmware version: %d.%d\n",
cap.tpm_version.Major,
@@ -236,7 +297,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
cap.tpm_version.revMinor);
}
- return str - buf;
+ rc = str - buf;
+put_ops:
+ tpm_put_ops(chip);
+ return rc;
}
static DEVICE_ATTR_RO(caps);
@@ -244,10 +308,17 @@ static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ ssize_t rc;
+
if (chip == NULL)
return 0;
+ rc = tpm_try_get_ops(chip);
+ if (rc)
+ return rc;
+
chip->ops->cancel(chip);
+ tpm_put_ops(chip);
return count;
}
static DEVICE_ATTR_WO(cancel);
@@ -304,16 +375,6 @@ static const struct attribute_group tpm_dev_group = {
void tpm_sysfs_add_device(struct tpm_chip *chip)
{
- /* XXX: If you wish to remove this restriction, you must first update
- * tpm_sysfs to explicitly lock chip->ops.
- */
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- return;
-
- /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
- * is called before ops is null'd and the sysfs core synchronizes this
- * removal so that no callbacks are running or can run again
- */
WARN_ON(chip->groups_cnt != 0);
chip->groups[chip->groups_cnt++] = &tpm_dev_group;
}
--
2.7.4