On Sat, Oct 01, 2016 at 03:01:25PM +0300, Jarkko Sakkinen wrote:
> On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote:
> > Currently, the securityfs pseudo files for obtaining the firmware
> > event log are created whether the event log properties exist or not.
> > This patch creates ascii and bios measurements pseudo files
> > only if read_log() is successful.
>
> Re-reviewing this. The commit message should mention about preventing
> a race condition.
>
> I think Jason was right. It makes code much more manageable with a
> small price of memory consumption.
>
> > Suggested-by: Jason Gunthorpe <jguntho...@obsidianresearch.com>
> > Signed-off-by: Nayna Jain <na...@linux.vnet.ibm.com>
> > ---
> > drivers/char/tpm/tpm.h | 6 +++++
> > drivers/char/tpm/tpm_acpi.c | 12 +++++++---
> > drivers/char/tpm/tpm_eventlog.c | 53
> > +++++++++++++++++++----------------------
> > drivers/char/tpm/tpm_eventlog.h | 7 +++++-
> > drivers/char/tpm/tpm_of.c | 4 +++-
> > 5 files changed, 48 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index b5866bb..68630cd 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,
> > @@ -156,6 +158,10 @@ struct tpm_chip {
> > struct rw_semaphore ops_sem;
> > const struct tpm_class_ops *ops;
> >
> > + struct tpm_bios_log log;
>
> struct tpm_bios_log should be renamed as struct tpm_event_log in some
> commit of this patch set as tpm_bios_log is a misleading name.
>
> > + struct tpm_securityfs_data bin_sfs_data;
> > + struct tpm_securityfs_data ascii_sfs_data;
>
> I think this is otherwise right but the struct name is very clunky.
> First of all it doesn't own the data and IMHO now it kind of implies
> of owning.
>
> Maybe something like tpm_event_log_fd would a better name. It's a
> description of the event log file essentially.
That's not a good name either because who knows if we have
new files there at some point. I would propose to use simply
struct tpmfs_fd for this data type.
Then the declariots would be simply:
struct tpmfs_fd binary_measurements_fd;
struct tpmfs_fd ascii_measurements_fd;
I think here the long descriptive names would be good use because
these fields are not heavily used in the soure code.
/Jarkko
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
