linux-kernel - Re: [PATCH v3 2/4] tpm: Implement tpm_chip_find() for other subsystems to find a TPM chip

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180622211346.GF19151@ziepe.ca>
Date:   Fri, 22 Jun 2018 15:13:46 -0600
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Stefan Berger <stefanb@...ux.vnet.ibm.com>
Cc:     linux-integrity@...r.kernel.org, jarkko.sakkinen@...ux.intel.com,
        zohar@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/4] tpm: Implement tpm_chip_find() for other
 subsystems to find a TPM chip

On Fri, Jun 22, 2018 at 04:45:47PM -0400, Stefan Berger wrote:
> On 06/22/2018 04:43 PM, Jason Gunthorpe wrote:
> >On Fri, Jun 22, 2018 at 12:46:11PM -0400, Stefan Berger wrote:
> >>Implement tpm_chip_find() for other subsystems to find a TPM chip and
> >>get a reference to that chip.
> >>
> >>Signed-off-by: Stefan Berger <stefanb@...ux.vnet.ibm.com>
> >>  drivers/char/tpm/tpm-chip.c | 27 +++++++++++++++++++++++++++
> >>  include/linux/tpm.h         |  5 +++++
> >>  2 files changed, 32 insertions(+)
> >>
> >>diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >>index 4e83695af068..2520555b1e17 100644
> >>+++ b/drivers/char/tpm/tpm-chip.c
> >>@@ -81,6 +81,33 @@ void tpm_put_ops(struct tpm_chip *chip)
> >>  EXPORT_SYMBOL_GPL(tpm_put_ops);
> >>  /**
> >>+ * tpm_chip_find() - find a TPM chip and get a reference to it
> >>+ */
> >>+struct tpm_chip *tpm_chip_find(void)
> >I feel like this should be called 'tpm_default_chip()'
> >
> >>+{
> >>+	struct tpm_chip *chip, *res = NULL;
> >>+	int chip_num = 0;
> >>+	int chip_prev;
> >>+
> >>+	mutex_lock(&idr_lock);
> >>+
> >>+	do {
> >>+		chip_prev = chip_num;
> >>+		chip = idr_get_next(&dev_nums_idr, &chip_num);
> >>+		if (chip) {
> >>+			get_device(&chip->dev);
> >>+			res = chip;
> >>+			break;
> >>+		}
> >>+	} while (chip_prev != chip_num);
> >>+
> >>+	mutex_unlock(&idr_lock);
> >And what was tpm_chip_find_get should just call this function..
> And then after that each time tpm_get_ops() ?

It is best to keep the tpm_chip_find_get/tpm_put_ops for internal use
as it also manages the kref lifetime of chip in a subtle way, it
relies on the ops lock not the kref to keep the memory valid when it
has a NULL parameter..

Something like this:

struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip)
{
    int rc;

    if (chip) {
          if (!tpm_try_get_ops(chip))
	      return NULL;
	  return chip;
    }

    chip = tpm_default_chip();
    rc = tpm_try_get_ops(chip));
    put_device(&chip->dev);

    if (rc)
           return NULL;
    return chip;
}

Jason