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