Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1498768

Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir with a static array

Show all headers | View raw

On Tue, Oct 11, 2016 at 12:23:15AM +0530, Nayna wrote:
> 
> 
> On 10/02/2016 12:55 AM, Jarkko Sakkinen wrote:
> >This commit is based on a commit by Nayna Jain. Replaced dynamically
> >allocated bios_dir with a static array as the size is always constant.
> >
> >Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >---
> >  drivers/char/tpm/tpm-chip.c     |  9 +++---
> >  drivers/char/tpm/tpm.h          |  3 +-
> >  drivers/char/tpm/tpm_eventlog.c | 62 +++++++++++++++++++----------------------
> >  drivers/char/tpm/tpm_eventlog.h | 10 +++----
> >  4 files changed, 41 insertions(+), 43 deletions(-)
> >
> >diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >index e595013..a56b609 100644
> >--- a/drivers/char/tpm/tpm-chip.c
> >+++ b/drivers/char/tpm/tpm-chip.c
> >@@ -278,14 +278,16 @@ static void tpm_del_char_device(struct tpm_chip *chip)
> >
> >  static int tpm1_chip_register(struct tpm_chip *chip)
> >  {
> >+	int rc;
> >+
> >  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >  		return 0;
> >
> >  	tpm_sysfs_add_device(chip);
> >
> >-	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
> >+	rc = tpm_bios_log_setup(chip);
> >
> >-	return 0;
> >+	return rc;
> >  }
> >
> >  static void tpm1_chip_unregister(struct tpm_chip *chip)
> >@@ -293,8 +295,7 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
> >  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >  		return;
> >
> >-	if (chip->bios_dir)
> >-		tpm_bios_log_teardown(chip->bios_dir);
> >+	tpm_bios_log_teardown(chip);
> >  }
> >
> >  static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> >diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> >index 4d183c9..4c118a4 100644
> >--- a/drivers/char/tpm/tpm.h
> >+++ b/drivers/char/tpm/tpm.h
> >@@ -40,6 +40,7 @@ enum tpm_const {
> >  	TPM_BUFSIZE = 4096,
> >  	TPM_NUM_DEVICES = 65536,
> >  	TPM_RETRY = 50,		/* 5 seconds */
> >+	TPM_NUM_EVENT_LOG_FILES = 3,
> >  };
> >
> >  enum tpm_timeout {
> >@@ -171,7 +172,7 @@ struct tpm_chip {
> >  	unsigned long duration[3]; /* jiffies */
> >  	bool duration_adjusted;
> >
> >-	struct dentry **bios_dir;
> >+	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
> >
> >  	const struct attribute_group *groups[3];
> >  	unsigned int groups_cnt;
> >diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> >index 75e6644..f5e1d06 100644
> >--- a/drivers/char/tpm/tpm_eventlog.c
> >+++ b/drivers/char/tpm/tpm_eventlog.c
> >@@ -368,54 +368,50 @@ static int is_bad(void *p)
> >  	return 0;
> >  }
> >
> >-struct dentry **tpm_bios_log_setup(const char *name)
> >+int tpm_bios_log_setup(struct tpm_chip *chip)
> >  {
> >-	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
> >+	const char *name = dev_name(&chip->dev);
> >+	unsigned int cnt;
> >
> >-	tpm_dir = securityfs_create_dir(name, NULL);
> >-	if (is_bad(tpm_dir))
> >-		goto out;
> >+	cnt = 0;
> >+	chip->bios_dir[cnt] =
> >+		securityfs_create_dir(name, NULL);
> >+	if (is_bad(chip->bios_dir[cnt]))
> >+		goto err;
> >+	cnt++;
> >
> >-	bin_file =
> >+	chip->bios_dir[cnt] =
> >  	    securityfs_create_file("binary_bios_measurements",
> >-				   S_IRUSR | S_IRGRP, tpm_dir,
> >+				   S_IRUSR | S_IRGRP, chip->bios_dir[0],
> >  				   (void *)&tpm_binary_b_measurments_seqops,
> >  				   &tpm_bios_measurements_ops);
> >-	if (is_bad(bin_file))
> >-		goto out_tpm;
> >+	if (is_bad(chip->bios_dir[cnt]))
> >+		goto err;
> >+	cnt++;
> >
> >-	ascii_file =
> >+	chip->bios_dir[cnt] =
> >  	    securityfs_create_file("ascii_bios_measurements",
> >-				   S_IRUSR | S_IRGRP, tpm_dir,
> >+				   S_IRUSR | S_IRGRP, chip->bios_dir[0],
> >  				   (void *)&tpm_ascii_b_measurments_seqops,
> >  				   &tpm_bios_measurements_ops);
> >-	if (is_bad(ascii_file))
> >-		goto out_bin;
> >+	if (is_bad(chip->bios_dir[cnt]))
> >+		goto err;
> >+	cnt++;
> >
> >-	ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
> >-	if (!ret)
> >-		goto out_ascii;
> >-
> >-	ret[0] = ascii_file;
> >-	ret[1] = bin_file;
> >-	ret[2] = tpm_dir;
> >-
> >-	return ret;
> >+	return 0;
> >
> >-out_ascii:
> >-	securityfs_remove(ascii_file);
> >-out_bin:
> >-	securityfs_remove(bin_file);
> >-out_tpm:
> >-	securityfs_remove(tpm_dir);
> >-out:
> >-	return NULL;
> >+err:
> >+	chip->bios_dir[cnt] = NULL;
> 
> The updated patch looks fine.
> Just, I am not sure if NULL assignment is needed.

It's not needed.

> >+	tpm_bios_log_teardown(chip);
> >+	return -EIO;
> >  }
> >
> >-void tpm_bios_log_teardown(struct dentry **lst)
> >+void tpm_bios_log_teardown(struct tpm_chip *chip)
> >  {
> >  	int i;
> >
> >-	for (i = 0; i < 3; i++)
> >-		securityfs_remove(lst[i]);
> >+	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
> >+		if (chip->bios_dir[i])
> 
> Probably, this check is not required because securityfs_remove() takes care
> of checking both NULL and err dentry.

Thanks. I did the adjustments to the eventlog branch. Please use these
three patches as "head" for your series. I'll apply the rest of patches
to this branch when you post the next version of the series.

Please also include these three patches to the next version when you
post even if they are applied there. The topic branch is there to make
the testing easier.

> Thanks & Regards,
>    - Nayna

/Jarkko

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH RESEND 0/3] Clean up handling of event log files Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-01 21:30 +0200
  [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir with a static array Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-01 21:30 +0200
    Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-10-02 23:30 +0200
      Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-03 14:30 +0200
        Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-08 13:10 +0200
    Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Nayna <nayna@linux.vnet.ibm.com> - 2016-10-10 21:00 +0200
      Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-11 13:30 +0200
        Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Nayna <nayna@linux.vnet.ibm.com> - 2016-10-11 19:30 +0200
          Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-11 20:20 +0200
        Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-10-11 19:40 +0200
          Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir  with a static array Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-11 20:20 +0200
  [PATCH RESEND 3/3] tpm: drop tpm1_chip_register(/unregister) Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-01 21:30 +0200
  [PATCH RESEND 1/3] tpm: define a generic open() method for ascii & bios measurements Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-01 21:30 +0200
    Re: [PATCH RESEND 1/3] tpm: define a generic open() method for ascii  & bios measurements Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-08 13:10 +0200

csiph-web