| Message ID | 20181108141541.12832-14-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove nested TPM operations | expand |
On 11/8/18 9:15 AM, Jarkko Sakkinen wrote: > Use tpm_try_get_ops() in tpm-sysfs.c so that we can consider moving > other decorations (locking, localities, power management for example) > inside it. This direction can be of course taken only after other call > sites for tpm_transmit() have been treated in the same way. > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/tpm-sysfs.c | 123 ++++++++++++++++++++++------------- > 1 file changed, 78 insertions(+), 45 deletions(-) > > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c > index 03e704f99ed6..3733491671ca 100644 > --- a/drivers/char/tpm/tpm-sysfs.c > +++ b/drivers/char/tpm/tpm-sysfs.c > @@ -39,7 +39,6 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, > { > struct tpm_buf tpm_buf; > struct tpm_readpubek_out *out; > - ssize_t rc; > int i; > char *str = buf; > struct tpm_chip *chip = to_tpm_chip(dev); > @@ -47,17 +46,17 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, > > memset(&anti_replay, 0, sizeof(anti_replay)); > > - if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK)) > + if (tpm_try_get_ops(chip)) > return 0; > > + if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK)) > + goto out_ops; > + > tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay)); > > - rc = tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, > - 0, "attempting to read the PUBEK"); > - if (rc) { > - tpm_buf_destroy(&tpm_buf); > - return 0; > - } > + if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, > + 0, "attempting to read the PUBEK")) > + goto out_buf; > > out = (struct tpm_readpubek_out *)&tpm_buf.data[10]; > str += > @@ -88,9 +87,11 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, > str += sprintf(str, "\n"); > } > > - rc = str - buf; > +out_buf: > tpm_buf_destroy(&tpm_buf); > - return rc; > +out_ops: > + tpm_put_ops(chip); > + return str - buf; > } > static DEVICE_ATTR_RO(pubek); > > @@ -103,10 +104,15 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr, > char *str = buf; > struct tpm_chip *chip = to_tpm_chip(dev); > > + if (tpm_try_get_ops(chip)) > + return 0; > + > if (tpm1_getcap(chip, TPM_CAP_PROP_PCR, &cap, > "attempting to determine the number of PCRS", > - sizeof(cap.num_pcrs))) > + sizeof(cap.num_pcrs))) { > + tpm_put_ops(chip); > return 0; > + } > > num_pcrs = be32_to_cpu(cap.num_pcrs); > for (i = 0; i < num_pcrs; i++) { > @@ -119,6 +125,7 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr, > str += sprintf(str, "%02X ", digest[j]); > str += sprintf(str, "\n"); > } > + tpm_put_ops(chip); > return str - buf; > } > static DEVICE_ATTR_RO(pcrs); > @@ -126,16 +133,21 @@ 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); > + ssize_t rc = 0; > cap_t cap; > - ssize_t rc; > > - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap, > - "attempting to determine the permanent enabled state", > - sizeof(cap.perm_flags)); > - if (rc) > + if (tpm_try_get_ops(chip)) > return 0; > > + if (tpm1_getcap(chip, TPM_CAP_FLAG_PERM, &cap, > + "attempting to determine the permanent enabled state", > + sizeof(cap.perm_flags))) > + goto out_ops; > + > rc = sprintf(buf, "%d\n", !cap.perm_flags.disable); > +out_ops: > + tpm_put_ops(chip); > return rc; > } > static DEVICE_ATTR_RO(enabled); > @@ -143,16 +155,21 @@ 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); > + ssize_t rc = 0; > cap_t cap; > - ssize_t rc; > > - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap, > - "attempting to determine the permanent active state", > - sizeof(cap.perm_flags)); > - if (rc) > + if (tpm_try_get_ops(chip)) > return 0; > > + if (tpm1_getcap(chip, TPM_CAP_FLAG_PERM, &cap, > + "attempting to determine the permanent active state", > + sizeof(cap.perm_flags))) > + goto out_ops; > + > rc = sprintf(buf, "%d\n", !cap.perm_flags.deactivated); > +out_ops: > + tpm_put_ops(chip); > return rc; > } > static DEVICE_ATTR_RO(active); > @@ -160,16 +177,21 @@ 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); > + ssize_t rc = 0; > cap_t cap; > - ssize_t rc; > > - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap, > - "attempting to determine the owner state", > - sizeof(cap.owned)); > - if (rc) > + if (tpm_try_get_ops(chip)) > return 0; > > + if (tpm1_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap, > + "attempting to determine the owner state", > + sizeof(cap.owned))) > + goto out_ops; > + > rc = sprintf(buf, "%d\n", cap.owned); > +out_ops: > + tpm_put_ops(chip); > return rc; > } > static DEVICE_ATTR_RO(owned); > @@ -177,16 +199,21 @@ 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); > + ssize_t rc = 0; > cap_t cap; > - ssize_t rc; > > - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap, > - "attempting to determine the temporary state", > - sizeof(cap.stclear_flags)); > - if (rc) > + if (tpm_try_get_ops(chip)) > return 0; > > + if (tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap, > + "attempting to determine the temporary state", > + sizeof(cap.stclear_flags))) > + goto out_ops; > + > rc = sprintf(buf, "%d\n", cap.stclear_flags.deactivated); > +out_ops: > + tpm_put_ops(chip); > return rc; > } > static DEVICE_ATTR_RO(temp_deactivated); > @@ -195,15 +222,18 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > struct tpm_chip *chip = to_tpm_chip(dev); > - cap_t cap; > - ssize_t rc; > + ssize_t rc = 0; > char *str = buf; > + cap_t cap; > > - rc = tpm1_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap, > - "attempting to determine the manufacturer", > - sizeof(cap.manufacturer_id)); > - if (rc) > + if (tpm_try_get_ops(chip)) > return 0; > + > + if (tpm1_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap, > + "attempting to determine the manufacturer", > + sizeof(cap.manufacturer_id))) > + goto out_ops; > + > str += sprintf(str, "Manufacturer: 0x%x\n", > be32_to_cpu(cap.manufacturer_id)); > > @@ -220,11 +250,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr, > cap.tpm_version_1_2.revMinor); > } else { > /* Otherwise just use TPM_STRUCT_VER */ > - rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap, > - "attempting to determine the 1.1 version", > - sizeof(cap.tpm_version)); > - if (rc) > - return 0; > + if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap, > + "attempting to determine the 1.1 version", > + sizeof(cap.tpm_version))) > + goto out_ops; > str += sprintf(str, > "TCG version: %d.%d\nFirmware version: %d.%d\n", > cap.tpm_version.Major, > @@ -232,8 +261,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr, > cap.tpm_version.revMajor, > cap.tpm_version.revMinor); > } > - > - return str - buf; > + rc = str - buf; > +out_ops: > + tpm_put_ops(chip); > + return rc; > } > static DEVICE_ATTR_RO(caps); > > @@ -241,10 +272,12 @@ 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); > - if (chip == NULL) > + > + if (tpm_try_get_ops(chip)) > return 0; > > chip->ops->cancel(chip); > + tpm_put_ops(chip); > return count; > } > static DEVICE_ATTR_WO(cancel);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c index 03e704f99ed6..3733491671ca 100644 --- a/drivers/char/tpm/tpm-sysfs.c +++ b/drivers/char/tpm/tpm-sysfs.c @@ -39,7 +39,6 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, { struct tpm_buf tpm_buf; struct tpm_readpubek_out *out; - ssize_t rc; int i; char *str = buf; struct tpm_chip *chip = to_tpm_chip(dev); @@ -47,17 +46,17 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, memset(&anti_replay, 0, sizeof(anti_replay)); - if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK)) + if (tpm_try_get_ops(chip)) return 0; + if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK)) + goto out_ops; + tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay)); - rc = tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, - 0, "attempting to read the PUBEK"); - if (rc) { - tpm_buf_destroy(&tpm_buf); - return 0; - } + if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, + 0, "attempting to read the PUBEK")) + goto out_buf; out = (struct tpm_readpubek_out *)&tpm_buf.data[10]; str += @@ -88,9 +87,11 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, str += sprintf(str, "\n"); } - rc = str - buf; +out_buf: tpm_buf_destroy(&tpm_buf); - return rc; +out_ops: + tpm_put_ops(chip); + return str - buf; } static DEVICE_ATTR_RO(pubek); @@ -103,10 +104,15 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr, char *str = buf; struct tpm_chip *chip = to_tpm_chip(dev); + if (tpm_try_get_ops(chip)) + return 0; + if (tpm1_getcap(chip, TPM_CAP_PROP_PCR, &cap, "attempting to determine the number of PCRS", - sizeof(cap.num_pcrs))) + sizeof(cap.num_pcrs))) { + tpm_put_ops(chip); return 0; + } num_pcrs = be32_to_cpu(cap.num_pcrs); for (i = 0; i < num_pcrs; i++) { @@ -119,6 +125,7 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr, str += sprintf(str, "%02X ", digest[j]); str += sprintf(str, "\n"); } + tpm_put_ops(chip); return str - buf; } static DEVICE_ATTR_RO(pcrs); @@ -126,16 +133,21 @@ 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); + ssize_t rc = 0; cap_t cap; - ssize_t rc; - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap, - "attempting to determine the permanent enabled state", - sizeof(cap.perm_flags)); - if (rc) + if (tpm_try_get_ops(chip)) return 0; + if (tpm1_getcap(chip, TPM_CAP_FLAG_PERM, &cap, + "attempting to determine the permanent enabled state", + sizeof(cap.perm_flags))) + goto out_ops; + rc = sprintf(buf, "%d\n", !cap.perm_flags.disable); +out_ops: + tpm_put_ops(chip); return rc; } static DEVICE_ATTR_RO(enabled); @@ -143,16 +155,21 @@ 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); + ssize_t rc = 0; cap_t cap; - ssize_t rc; - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap, - "attempting to determine the permanent active state", - sizeof(cap.perm_flags)); - if (rc) + if (tpm_try_get_ops(chip)) return 0; + if (tpm1_getcap(chip, TPM_CAP_FLAG_PERM, &cap, + "attempting to determine the permanent active state", + sizeof(cap.perm_flags))) + goto out_ops; + rc = sprintf(buf, "%d\n", !cap.perm_flags.deactivated); +out_ops: + tpm_put_ops(chip); return rc; } static DEVICE_ATTR_RO(active); @@ -160,16 +177,21 @@ 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); + ssize_t rc = 0; cap_t cap; - ssize_t rc; - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap, - "attempting to determine the owner state", - sizeof(cap.owned)); - if (rc) + if (tpm_try_get_ops(chip)) return 0; + if (tpm1_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap, + "attempting to determine the owner state", + sizeof(cap.owned))) + goto out_ops; + rc = sprintf(buf, "%d\n", cap.owned); +out_ops: + tpm_put_ops(chip); return rc; } static DEVICE_ATTR_RO(owned); @@ -177,16 +199,21 @@ 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); + ssize_t rc = 0; cap_t cap; - ssize_t rc; - rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap, - "attempting to determine the temporary state", - sizeof(cap.stclear_flags)); - if (rc) + if (tpm_try_get_ops(chip)) return 0; + if (tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap, + "attempting to determine the temporary state", + sizeof(cap.stclear_flags))) + goto out_ops; + rc = sprintf(buf, "%d\n", cap.stclear_flags.deactivated); +out_ops: + tpm_put_ops(chip); return rc; } static DEVICE_ATTR_RO(temp_deactivated); @@ -195,15 +222,18 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr, char *buf) { struct tpm_chip *chip = to_tpm_chip(dev); - cap_t cap; - ssize_t rc; + ssize_t rc = 0; char *str = buf; + cap_t cap; - rc = tpm1_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap, - "attempting to determine the manufacturer", - sizeof(cap.manufacturer_id)); - if (rc) + if (tpm_try_get_ops(chip)) return 0; + + if (tpm1_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap, + "attempting to determine the manufacturer", + sizeof(cap.manufacturer_id))) + goto out_ops; + str += sprintf(str, "Manufacturer: 0x%x\n", be32_to_cpu(cap.manufacturer_id)); @@ -220,11 +250,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr, cap.tpm_version_1_2.revMinor); } else { /* Otherwise just use TPM_STRUCT_VER */ - rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap, - "attempting to determine the 1.1 version", - sizeof(cap.tpm_version)); - if (rc) - return 0; + if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap, + "attempting to determine the 1.1 version", + sizeof(cap.tpm_version))) + goto out_ops; str += sprintf(str, "TCG version: %d.%d\nFirmware version: %d.%d\n", cap.tpm_version.Major, @@ -232,8 +261,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr, cap.tpm_version.revMajor, cap.tpm_version.revMinor); } - - return str - buf; + rc = str - buf; +out_ops: + tpm_put_ops(chip); + return rc; } static DEVICE_ATTR_RO(caps); @@ -241,10 +272,12 @@ 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); - if (chip == NULL) + + if (tpm_try_get_ops(chip)) return 0; chip->ops->cancel(chip); + tpm_put_ops(chip); return count; } static DEVICE_ATTR_WO(cancel);
Use tpm_try_get_ops() in tpm-sysfs.c so that we can consider moving other decorations (locking, localities, power management for example) inside it. This direction can be of course taken only after other call sites for tpm_transmit() have been treated in the same way. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- drivers/char/tpm/tpm-sysfs.c | 123 ++++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 45 deletions(-)