lkml.org 
[lkml]   [2016]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 5/9] tpm: have event log use the tpm_chip
    On Mon, Nov 14, 2016 at 05:00:52AM -0500, Nayna Jain wrote:
    > Move the backing memory for the event log into tpm_chip and push
    > the tpm_chip into read_log. This optimizes read_log processing by
    > only doing it once and prepares things for the next patches in the
    > series which require the tpm_chip to locate the event log via
    > ACPI and OF handles instead of searching.
    >
    > This is straightfoward except for the issue of passing a kref through
    > i_private with securityfs. Since securityfs_remove does not have any
    > removal fencing like sysfs we use the inode lock to safely get a
    > kref on the tpm_chip.
    >
    > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
    > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

    /Jarkko

    > ---
    > drivers/char/tpm/tpm-chip.c | 3 +-
    > drivers/char/tpm/tpm.h | 11 ++++++
    > drivers/char/tpm/tpm_acpi.c | 15 +++++--
    > drivers/char/tpm/tpm_eventlog.c | 88 ++++++++++++++++++++++++++---------------
    > drivers/char/tpm/tpm_eventlog.h | 2 +-
    > drivers/char/tpm/tpm_of.c | 4 +-
    > 6 files changed, 85 insertions(+), 38 deletions(-)
    >
    > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
    > index 250a651..3f27753 100644
    > --- a/drivers/char/tpm/tpm-chip.c
    > +++ b/drivers/char/tpm/tpm-chip.c
    > @@ -127,6 +127,7 @@ static void tpm_dev_release(struct device *dev)
    > idr_remove(&dev_nums_idr, chip->dev_num);
    > mutex_unlock(&idr_lock);
    >
    > + kfree(chip->log.bios_event_log);
    > kfree(chip);
    > }
    >
    > @@ -345,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip)
    > tpm_sysfs_add_device(chip);
    >
    > rc = tpm_bios_log_setup(chip);
    > - if (rc)
    > + if (rc == -ENODEV)
    > return rc;
    >
    > tpm_add_ppi(chip);
    > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
    > index 9d69580..1ae9768 100644
    > --- a/drivers/char/tpm/tpm.h
    > +++ b/drivers/char/tpm/tpm.h
    > @@ -35,6 +35,8 @@
    > #include <linux/cdev.h>
    > #include <linux/highmem.h>
    >
    > +#include "tpm_eventlog.h"
    > +
    > enum tpm_const {
    > TPM_MINOR = 224, /* officially assigned */
    > TPM_BUFSIZE = 4096,
    > @@ -146,6 +148,11 @@ enum tpm_chip_flags {
    > TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
    > };
    >
    > +struct tpm_chip_seqops {
    > + struct tpm_chip *chip;
    > + const struct seq_operations *seqops;
    > +};
    > +
    > struct tpm_chip {
    > struct device dev;
    > struct cdev cdev;
    > @@ -157,6 +164,10 @@ struct tpm_chip {
    > struct rw_semaphore ops_sem;
    > const struct tpm_class_ops *ops;
    >
    > + struct tpm_bios_log log;
    > + struct tpm_chip_seqops bin_log_seqops;
    > + struct tpm_chip_seqops ascii_log_seqops;
    > +
    > unsigned int flags;
    >
    > int dev_num; /* /dev/tpm# */
    > diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
    > index 565a947..01dfb35 100644
    > --- a/drivers/char/tpm/tpm_acpi.c
    > +++ b/drivers/char/tpm/tpm_acpi.c
    > @@ -9,7 +9,7 @@
    > *
    > * Maintained by: <tpmdd-devel@lists.sourceforge.net>
    > *
    > - * Access to the eventlog extended by the TCG BIOS of PC platform
    > + * Access to the event log extended by the TCG BIOS of PC platform
    > *
    > * This program is free software; you can redistribute it and/or
    > * modify it under the terms of the GNU General Public License
    > @@ -45,13 +45,15 @@ struct acpi_tcpa {
    > };
    >
    > /* read binary bios log */
    > -int read_log(struct tpm_bios_log *log)
    > +int read_log(struct tpm_chip *chip)
    > {
    > struct acpi_tcpa *buff;
    > acpi_status status;
    > void __iomem *virt;
    > u64 len, start;
    > + struct tpm_bios_log *log;
    >
    > + log = &chip->log;
    > if (log->bios_event_log != NULL) {
    > printk(KERN_ERR
    > "%s: ERROR - Eventlog already initialized\n",
    > @@ -97,13 +99,18 @@ int read_log(struct tpm_bios_log *log)
    >
    > virt = acpi_os_map_iomem(start, len);
    > if (!virt) {
    > - kfree(log->bios_event_log);
    > printk("%s: ERROR - Unable to map memory\n", __func__);
    > - return -EIO;
    > + goto err;
    > }
    >
    > memcpy_fromio(log->bios_event_log, virt, len);
    >
    > acpi_os_unmap_iomem(virt, len);
    > return 0;
    > +
    > +err:
    > + kfree(log->bios_event_log);
    > + log->bios_event_log = NULL;
    > + return -EIO;
    > +
    > }
    > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
    > index 57ac862..f8c42fe 100644
    > --- a/drivers/char/tpm/tpm_eventlog.c
    > +++ b/drivers/char/tpm/tpm_eventlog.c
    > @@ -11,7 +11,7 @@
    > *
    > * Maintained by: <tpmdd-devel@lists.sourceforge.net>
    > *
    > - * Access to the eventlog created by a system's firmware / BIOS
    > + * Access to the event log created by a system's firmware / BIOS
    > *
    > * This program is free software; you can redistribute it and/or
    > * modify it under the terms of the GNU General Public License
    > @@ -73,7 +73,8 @@ static const char* tcpa_pc_event_id_strings[] = {
    > static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
    > {
    > loff_t i;
    > - struct tpm_bios_log *log = m->private;
    > + struct tpm_chip *chip = m->private;
    > + struct tpm_bios_log *log = &chip->log;
    > void *addr = log->bios_event_log;
    > void *limit = log->bios_event_log_end;
    > struct tcpa_event *event;
    > @@ -120,7 +121,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
    > loff_t *pos)
    > {
    > struct tcpa_event *event = v;
    > - struct tpm_bios_log *log = m->private;
    > + struct tpm_chip *chip = m->private;
    > + struct tpm_bios_log *log = &chip->log;
    > void *limit = log->bios_event_log_end;
    > u32 converted_event_size;
    > u32 converted_event_type;
    > @@ -261,13 +263,10 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
    > static int tpm_bios_measurements_release(struct inode *inode,
    > struct file *file)
    > {
    > - struct seq_file *seq = file->private_data;
    > - struct tpm_bios_log *log = seq->private;
    > + struct seq_file *seq = (struct seq_file *)file->private_data;
    > + struct tpm_chip *chip = (struct tpm_chip *)seq->private;
    >
    > - if (log) {
    > - kfree(log->bios_event_log);
    > - kfree(log);
    > - }
    > + put_device(&chip->dev);
    >
    > return seq_release(inode, file);
    > }
    > @@ -323,33 +322,30 @@ static int tpm_bios_measurements_open(struct inode *inode,
    > struct file *file)
    > {
    > int err;
    > - struct tpm_bios_log *log;
    > struct seq_file *seq;
    > - const struct seq_operations *seqops =
    > - (const struct seq_operations *)inode->i_private;
    > -
    > - log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
    > - if (!log)
    > - return -ENOMEM;
    > -
    > - if ((err = read_log(log)))
    > - goto out_free;
    > + struct tpm_chip_seqops *chip_seqops;
    > + const struct seq_operations *seqops;
    > + struct tpm_chip *chip;
    > +
    > + inode_lock(inode);
    > + if (!inode->i_private) {
    > + inode_unlock(inode);
    > + return -ENODEV;
    > + }
    > + chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
    > + seqops = chip_seqops->seqops;
    > + chip = chip_seqops->chip;
    > + get_device(&chip->dev);
    > + inode_unlock(inode);
    >
    > /* now register seq file */
    > err = seq_open(file, seqops);
    > if (!err) {
    > seq = file->private_data;
    > - seq->private = log;
    > - } else {
    > - goto out_free;
    > + seq->private = chip;
    > }
    >
    > -out:
    > return err;
    > -out_free:
    > - kfree(log->bios_event_log);
    > - kfree(log);
    > - goto out;
    > }
    >
    > static const struct file_operations tpm_bios_measurements_ops = {
    > @@ -372,29 +368,47 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
    > {
    > const char *name = dev_name(&chip->dev);
    > unsigned int cnt;
    > + int rc = 0;
    >
    > if (chip->flags & TPM_CHIP_FLAG_TPM2)
    > return 0;
    >
    > + rc = read_log(chip);
    > + /*
    > + * read_log failure means event log is not supported except for ENOMEM.
    > + */
    > + if (rc < 0) {
    > + if (rc == -ENOMEM)
    > + return -ENODEV;
    > + else
    > + return rc;
    > + }
    > +
    > cnt = 0;
    > chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
    > if (is_bad(chip->bios_dir[cnt]))
    > goto err;
    > cnt++;
    >
    > + chip->bin_log_seqops.chip = chip;
    > + chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
    > +
    > chip->bios_dir[cnt] =
    > securityfs_create_file("binary_bios_measurements",
    > 0440, chip->bios_dir[0],
    > - (void *)&tpm_binary_b_measurements_seqops,
    > + (void *)&chip->bin_log_seqops,
    > &tpm_bios_measurements_ops);
    > if (is_bad(chip->bios_dir[cnt]))
    > goto err;
    > cnt++;
    >
    > + chip->ascii_log_seqops.chip = chip;
    > + chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
    > +
    > chip->bios_dir[cnt] =
    > securityfs_create_file("ascii_bios_measurements",
    > 0440, chip->bios_dir[0],
    > - (void *)&tpm_ascii_b_measurements_seqops,
    > + (void *)&chip->ascii_log_seqops,
    > &tpm_bios_measurements_ops);
    > if (is_bad(chip->bios_dir[cnt]))
    > goto err;
    > @@ -411,7 +425,19 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
    > void tpm_bios_log_teardown(struct tpm_chip *chip)
    > {
    > int i;
    > -
    > - for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
    > + struct inode *inode;
    > +
    > + /* securityfs_remove currently doesn't take care of handling sync
    > + * between removal and opening of pseudo files. To handle this, a
    > + * workaround is added by making i_private = NULL here during removal
    > + * and to check it during open(), both within inode_lock()/unlock().
    > + * This design ensures that open() either safely gets kref or fails.
    > + */
    > + for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
    > + inode = d_inode(chip->bios_dir[i]);
    > + inode_lock(inode);
    > + inode->i_private = NULL;
    > + inode_unlock(inode);
    > securityfs_remove(chip->bios_dir[i]);
    > + }
    > }
    > diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
    > index fd3357e..6df2f8e 100644
    > --- a/drivers/char/tpm/tpm_eventlog.h
    > +++ b/drivers/char/tpm/tpm_eventlog.h
    > @@ -73,7 +73,7 @@ enum tcpa_pc_event_ids {
    > HOST_TABLE_OF_DEVICES,
    > };
    >
    > -int read_log(struct tpm_bios_log *log);
    > +int read_log(struct tpm_chip *chip);
    >
    > #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
    > defined(CONFIG_ACPI)
    > diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
    > index 570f30c..68d891a 100644
    > --- a/drivers/char/tpm/tpm_of.c
    > +++ b/drivers/char/tpm/tpm_of.c
    > @@ -20,12 +20,14 @@
    > #include "tpm.h"
    > #include "tpm_eventlog.h"
    >
    > -int read_log(struct tpm_bios_log *log)
    > +int read_log(struct tpm_chip *chip)
    > {
    > struct device_node *np;
    > const u32 *sizep;
    > const u64 *basep;
    > + struct tpm_bios_log *log;
    >
    > + log = &chip->log;
    > if (log->bios_event_log != NULL) {
    > pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
    > return -EFAULT;
    > --
    > 2.5.0
    >

    \
     
     \ /
      Last update: 2016-11-15 00:44    [W:2.418 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site