| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B9D94EA09@hasmsx108.ger.corp.intel.com>
Date: Mon, 18 Jun 2018 19:24:39 +0000
From: "Winkler, Tomas" <tomas.winkler@...el.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
CC: Jason Gunthorpe <jgg@...pe.ca>,
"Usyskin, Alexander" <alexander.usyskin@...el.com>,
"linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
"linux-security-module@...r.kernel.org"
<linux-security-module@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v6] tpm: separate cmd_ready/go_idle from runtime_pm
>
> On Fri, Jun 08, 2018 at 12:20:40AM +0300, Tomas Winkler wrote:
> > We cannot use go_idle cmd_ready commands via runtime_pm handles as
> > with the introduction of localities this is no longer an optional
> > feature, while runtime pm can be not enabled.
> > Though cmd_ready/go_idle provides a power saving, it's also a part of
> > TPM2 protocol and should be called explicitly.
> > This patch exposes cmd_read/go_idle via tpm class ops and removes
> > runtime pm support as it is not used by any driver.
> >
> > A new tpm transmit flag is added TPM_TRANSMIT_NESTED, which implies
> > TPM_TRANSMIT_UNLOCKED and TPM_TRANSMIT_RAW. Both are needed
> to resolve
> > tpm spaces and locality request recursive calls to tpm_transmit().
> >
> > New wrappers are added tpm_cmd_ready() and tpm_go_idle() to
> streamline
> > tpm_try_transmit code.
> >
> > tpm_crb no longer needs own power saving functions and can drop using
> > tpm_pm_suspend/resume.
> >
> > This patch cannot be really separated from the locality fix.
> > Fixes: 888d867df441 (tpm: cmd_ready command can be issued only after
> > granting locality)
> >
> >
> > Cc: stable@...r.kernel.org
> > Fixes: 888d867df441 (tpm: cmd_ready command can be issued only after
> > granting locality)
> > Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> > ---
> > V2-3:resent
> > V4: 1. Use tpm_pm_suspend/resume in tpm_crb
> > 2. Drop cmd_ready/go_idle around tpm_chip_register, as there is no
> > usage counter like in runtime_pm.
> > V5: 1. add tpm_cmd_ready and tpm_go_idle wrappers.
> > 2. Abuse TPM_TRANSMIT_UNLOCKED flag to resolve tpm space
> > recursion.
> > V6: 1. Add new flags TPM_TRANSMIT_NESTED which implies
> > TPM_TRANSMIT_UNLOCKED and TPM_TRANSMIT_RAW, as not all
> > unlocked flows are recursive i.e. tpm2_unseal_trusted.
> >
> > drivers/char/tpm/tpm-interface.c | 51 +++++++++++++++----
> > drivers/char/tpm/tpm.h | 13 ++++-
> > drivers/char/tpm/tpm2-space.c | 12 ++---
> > drivers/char/tpm/tpm_crb.c | 101 ++++++++++----------------------------
> > drivers/char/tpm/tpm_vtpm_proxy.c | 2 +-
> > include/linux/tpm.h | 2 +
> > 6 files changed, 88 insertions(+), 93 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index e32f6e85dc6d..1abd8261651b 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -29,7 +29,6 @@
> > #include <linux/mutex.h>
> > #include <linux/spinlock.h>
> > #include <linux/freezer.h>
> > -#include <linux/pm_runtime.h>
> > #include <linux/tpm_eventlog.h>
> >
> > #include "tpm.h"
> > @@ -369,10 +368,13 @@ static int tpm_validate_command(struct
> tpm_chip *chip,
> > return -EINVAL;
> > }
> >
> > -static int tpm_request_locality(struct tpm_chip *chip)
> > +static int tpm_request_locality(struct tpm_chip *chip, unsigned int
> > +flags)
> > {
> > int rc;
> >
> > + if (flags & __TPM_TRANSMIT_RAW)
> > + return 0;
> > +
> > if (!chip->ops->request_locality)
> > return 0;
> >
> > @@ -385,10 +387,13 @@ static int tpm_request_locality(struct tpm_chip
> *chip)
> > return 0;
> > }
> >
> > -static void tpm_relinquish_locality(struct tpm_chip *chip)
> > +static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned
> > +int flags)
> > {
> > int rc;
> >
> > + if (flags & __TPM_TRANSMIT_RAW)
> > + return;
> > +
> > if (!chip->ops->relinquish_locality)
> > return;
> >
> > @@ -399,6 +404,28 @@ static void tpm_relinquish_locality(struct
> tpm_chip *chip)
> > chip->locality = -1;
> > }
> >
> > +static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags) {
> > + if (flags & __TPM_TRANSMIT_RAW)
> > + return 0;
> > +
> > + if (!chip->ops->cmd_ready)
> > + return 0;
> > +
> > + return chip->ops->cmd_ready(chip);
> > +}
> > +
> > +static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags) {
> > + if (flags & __TPM_TRANSMIT_RAW)
> > + return 0;
> > +
> > + if (!chip->ops->go_idle)
> > + return 0;
> > +
> > + return chip->ops->go_idle(chip);
> > +}
> > +
> > static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> > struct tpm_space *space,
> > u8 *buf, size_t bufsiz,
> > @@ -449,14 +476,15 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> *chip,
> > /* Store the decision as chip->locality will be changed. */
> > need_locality = chip->locality == -1;
> >
> > - if (!(flags & TPM_TRANSMIT_RAW) && need_locality) {
> > - rc = tpm_request_locality(chip);
> > + if (need_locality) {
> > + rc = tpm_request_locality(chip, flags);
> > if (rc < 0)
> > goto out_no_locality;
> > }
> >
> > - if (chip->dev.parent)
> > - pm_runtime_get_sync(chip->dev.parent);
> > + rc = tpm_cmd_ready(chip, flags);
> > + if (rc)
> > + goto out;
> >
> > rc = tpm2_prepare_space(chip, space, ordinal, buf);
> > if (rc)
> > @@ -516,13 +544,16 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> *chip,
> > }
> >
> > rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> > + if (rc)
> > + dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
> >
> > out:
> > - if (chip->dev.parent)
> > - pm_runtime_put_sync(chip->dev.parent);
> > + rc = tpm_go_idle(chip, flags);
> > + if (rc)
> > + goto out;
> >
> > if (need_locality)
> > - tpm_relinquish_locality(chip);
> > + tpm_relinquish_locality(chip, flags);
> >
> > out_no_locality:
> > if (chip->ops->clk_enable != NULL)
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> > 4426649e431c..beb0a763016c 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -511,9 +511,18 @@ extern const struct file_operations tpm_fops;
> > extern const struct file_operations tpmrm_fops; extern struct idr
> > dev_nums_idr;
> >
> > +/**
> > + * enum tpm_transmit_flags
> > + *
> > + * @TPM_TRANSMIT_UNLOCKED: used to lock sequence of tpm_transmit
> calls.
> > + * @__TPM_TRANSMIT_RAW: prevent recursive calls into setup steps
> > + * (go idle, locality,..). Don't use directly.
> > + * @TPM_TRANSMIT_NESTED: Use from nested tpm_transmit calls */
> > enum tpm_transmit_flags {
> > - TPM_TRANSMIT_UNLOCKED = BIT(0),
> > - TPM_TRANSMIT_RAW = BIT(1),
> > + TPM_TRANSMIT_UNLOCKED = BIT(0),
> > + __TPM_TRANSMIT_RAW = BIT(1),
>
> NAK for the naming convention.
What do you suggest
Tomas
Powered by blists - more mailing lists