Re: [tpmdd-devel] [PATCH] tpm: use test_bit() to check TPM2 flag in eventlog and sysfs code
On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote:
> On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
>> There is change done to introduce atomic bitops to set and test
>> chip->flags.
>> This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
>> to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
>>
>> Signed-off-by: Nayna Jain
>
> I'm bit lost of the purpose of this patch.
I was using tabrm branch which has changes related to using bitops for
chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup()
with the existing way of checking. Replacing existing one with
test_bit() check makes it work. Same in case of tpm_sysfs_add_device().
Thanks & Regards,
- Nayna
>
> /Jarkko
>
>> ---
>> drivers/char/tpm/tpm-sysfs.c| 2 +-
>> drivers/char/tpm/tpm_eventlog.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index 82298e51..9a37c26 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -284,7 +284,7 @@ static const struct attribute_group tpm_dev_group = {
>>
>> void tpm_sysfs_add_device(struct tpm_chip *chip)
>> {
>> -if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
>> return;
>>
>> /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
>> diff --git a/drivers/char/tpm/tpm_eventlog.c
>> b/drivers/char/tpm/tpm_eventlog.c
>> index ebec4ac..dede2ec 100644
>> --- a/drivers/char/tpm/tpm_eventlog.c
>> +++ b/drivers/char/tpm/tpm_eventlog.c
>> @@ -391,7 +391,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>> unsigned int cnt;
>> int rc = 0;
>>
>> -if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
>> return 0;
>>
>> rc = tpm_read_log(chip);
>> --
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-security-module" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH 2/2] tpm: Fix error code handling after tpm_bios_log_setup
On 11/21/2016 10:45 PM, Jason Gunthorpe wrote: > On Mon, Nov 21, 2016 at 12:06:20AM +0530, Nayna wrote: >>> rc = tpm_bios_log_setup(chip); >>> - if (rc == -ENODEV) >>> + if (rc != 0 && rc != -ENODEV) >>> return rc; >> >> This will return in case of -EFAULT as well, where the check is that log is >> already initialized. Do we want to fail the probe here as well ? >> >> -EFAULT is returned from tpm_read_log() as below: > > That is fine, we should never read the log twice. > >>> index fb603a74cbd29e..2a15b866ac257a 100644 >>> +++ b/drivers/char/tpm/tpm_eventlog.c >>> @@ -377,14 +377,21 @@ static int tpm_read_log(struct tpm_chip *chip) >>> } >>> >>> rc = tpm_read_log_acpi(chip); >> >> This is to understand.. >> It can return -ENOMEM error here, contd below... >> >>> - if ((rc == 0) || (rc == -ENOMEM)) >>> + if (rc != -ENODEV) >>> return rc; >>> >>> - rc = tpm_read_log_of(chip); >>> - >>> - return rc; >>> + return tpm_read_log_of(chip); >> >> So, in ACPI if -ENOMEM error is returned, it will continue to >> tpm_read_log_of(chip), which will return -ENODEV. So, -ENOMEM error is now >> masked with -ENODEV error. > > No, if acpi is -ENOMEM then 'if (rc != -ENODEV)' is true and it > returns -ENOMEM. Yeah, I missed it. Sorry. > >>> sizep = of_get_property(np, "linux,sml-size", NULL); >>> - if (sizep == NULL) >>> + basep = of_get_property(np, "linux,sml-base", NULL); >>> + if (sizep == NULL && basep == NULL) >>> + return -ENODEV; >>> + if (sizep == NULL || basep == NULL) >>> return -EIO; >> >> To confirm my understanding, >> >> For -ENODEV, it means that both properties are not supported, so event log >> is not supported. > > Yes > >> For -EIO , it means that event log is supported but there is some failure in >> getting one of them, so should fail the probe. >> Is my understanding right ? > > Yes Thanks for explaining. Looks good now. Thanks & Regards, - Nayna > > Jason > -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH RFC 2/2] tpm: refactor tpm2_get_tpm_pt to tpm2_getcap_cmd
On 11/18/2016 09:43 PM, Jarkko Sakkinen wrote: > On Fri, Nov 18, 2016 at 05:42:01PM +0530, Nayna wrote: >> >> >> On 11/17/2016 11:12 PM, Jarkko Sakkinen wrote: >>> On Thu, Nov 17, 2016 at 05:20:36PM +0530, Nayna wrote: >>> I tested this for capability TPM2_CAP_PCRS. It seems TPM2_CAP_PCRS capability always returns full PCR allocation, and more_data as 0, So, I think the idea of looping over based on more_data may not work for this capability. >>> >>> You can always request one value at a time until there's no more. >>> >>> If you request N values, depending on the hardware, the hardware returns >>> to you anything from 1 to N values. If you implement a function that >>> requests N values in the command, you *must* handle the case where >>> moreData is 1 even if the hardware you are testing that never happens. >>> >>> That's the reason why I would start with a function that you request one >>> property of one capability and optimize it in future if it doesn't scale >>> for some workload. >>> >>> Do you have a workload where it doesn't scale? >> >> Thanks Jarkko for explaining in detail. >> >> If I understood correctly, the idea is to request for one property at a >> time, and if we need multiple properties, then to request for each of them >> in a loop. In case of TPM2_CAP_PCRS, property is always zero. This is how I >> am calling getcap_cmd for TPM2_CAP_PCRS. >> >> tpm2_getcap_cmd(chip, TPM2_CAP_PCRS, 0, &cap_data, "get active pcr banks"); >> >> Output : >> >> [ 17.081665] tpm: cap id to receive value is 2 >> [ 17.081666] tpm: TPM2_CAP_COMMANDS: more data 1 >> [ 17.081667] tpm: 2 >> [ 17.081668] tpm: tpm2_get_active_banks ---> cap is TPM2_CAP_PCRS >> [ 17.171665] tpm: cap id to receive value is 5 >> [ 17.171666] tpm: TPM2_CAP_PCRS: more data 0 ---> more data is zero. >> [ 17.171666] tpm: TPM2_CAP_PCRS: more data 0 >> [ 17.171667] tpm: count pcr banks is 2 --> count of active pcr banks >> information returned >> >> more_data is always zero here, so am not sure how to handle more_data in >> this case ? >> Since property_id is always zero, I am not able to request for one property >> at a time. >> and response_buffer returns the details for both active banks. >> >> This is the expected behavior defined in TCG 2.0 Part 3 Commands >> Specification (Section 30.2.1): >> >> "TPM_CAP_PCRS – Returns the current allocation of PCR in a >> TPML_PCR_SELECTION. The property parameter shall be zero. The TPM will >> always respond to this command with the full PCR allocation and moreData >> will be NO." >> >> Please let me know, if I am missing something. > > Thanks for pointing that. I think you got it right and I had some wrong > assumptions about 'moreData'. > > Here's what I propose. Do a non-generic function just for getting CAP_PCRS. > You could call it tpm2_get_pcr_allocation() as you don't want or rather > need to handle all the bells and whistles in that TPM command. > > It makes a lot more sense now than having one-size-for-all function. Thanks Jarkko, Yeah, Sure, I will write it as different non-generic call. Otherwise, the function works good. Also, I am thinking now I can write "multi-bank support for extend" on top of master branch itself. Any issues with that ? Thanks & Regards, - Nayna > > /Jarkko > -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH 1/4] tpm: add kdoc for tpm_transmit and tpm_transmit_cmd
>
> On Wed, Nov 16, 2016 at 07:03:38PM +0200, Tomas Winkler wrote:
> > Functions tpm_transmit and transmit_cmd are referenced from other
> > functions kdoc hence deserve documentation.
> >
> > Signed-off-by: Tomas Winkler
>
> Do you know how to make "make htmldocs" to generate documentation for the
> source tree? I do not except the ones that I imported when I created an RST
> version of the Stefans documentation.
I'm using a quick script like that
RES=tpm-kdoc.html
rm -r $RES kdoc.err
files=$(git ls-files drivers/char/tpm/*.[ch] includ/linux/tpm.h
includ/linux/tpm_command.h)
echo '' > $RES
for f in ${files}; do
./scripts/kernel-doc -html $f >> $RES 2>> kdoc.err
done
echo '' >> $RES
cat kdoc.err | grep -v 'warning: no structured comments found'
>
> I'm mainly concerned having one line per error code. My guess it that Sphinx
> will remove the newline characters.
Why do you find it tragic that the new line is removed, we just need to make
sure it's readable in both cases.
> The way I sorted that out in Stefans documentation was '|' prefix on the
> beginning of line, which does newline enforcement.
>
> So you would have something along the lines:
>
> * | 0 when the operation is successful
> * | negative number for system errors (errno)
> * | A positive number for a TPM error.
>
> It's uglier in unrendered form but still kind of sustainable and in the
> rendered
> output things won't fall apart...
I'm not sure it matters much regarding the readability of the document but the
old html format will have then those strange 'I' rendered.
Let me know which why you won't to go frankly I don't have strong opinion
either way, I will respin the patch anyhow as I've missed some '.'
Thanks
Tomas
> /Jarkko
>
> > ---
> > drivers/char/tpm/tpm-interface.c | 33
> > -
> > 1 file changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index a2688ac2b48f..56c238a6e005 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -328,8 +328,17 @@ unsigned long tpm_calc_ordinal_duration(struct
> > tpm_chip *chip, } EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
> >
> > -/*
> > - * Internal kernel interface to transmit TPM commands
> > +/**
> > + * tmp_transmit - Internal kernel interface to transmit TPM commands.
> > + *
> > + * @chip: TPM chip to use
> > + * @buf: TPM command buffer
> > + * @bufsiz: length of the TPM command buffer
> > + * @flags: tpm transmit flags - bitmap
> > + *
> > + * Return:
> > + * 0 when the operation is successful
> > + * A negative number for system errors (errno)
> > */
> > ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
> > unsigned int flags)
> > @@ -409,9 +418,21 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const
> u8 *buf, size_t bufsiz,
> > return rc;
> > }
> >
> > -#define TPM_DIGEST_SIZE 20
> > -#define TPM_RET_CODE_IDX 6
> > -
> > +/**
> > + * tmp_transmit_cmd - send a tpm command to the device
> > + *The function extracts tpm out header return code
> > + *
> > + * @chip: TPM chip to use
> > + * @cmd: TPM command buffer
> > + * @len: length of the TPM command
> > + * @flags: tpm transmit flags - bitmap
> > + * @desc: command description used in the error message
> > + *
> > + * Return:
> > + * 0 when the operation is successful
> > + * A negative number for system errors (errno)
> > + * A positive number for a TPM error.
> > + */
> > ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
> > int len, unsigned int flags, const char *desc) { @@ -
> 434,6
> > +455,8 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void
> *cmd,
> > return err;
> > }
> >
> > +#define TPM_DIGEST_SIZE 20
> > +#define TPM_RET_CODE_IDX 6
> > #define TPM_INTERNAL_RESULT_SIZE 200
> > #define TPM_ORD_GET_CAP cpu_to_be32(101) #define
> TPM_ORD_GET_RANDOM
> > cpu_to_be32(70)
> > --
> > 2.7.4
> >
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH 1/4] tpm: add kdoc for tpm_transmit and tpm_transmit_cmd
On Tue, Nov 22, 2016 at 09:44:20AM +, Winkler, Tomas wrote:
> >
> > On Wed, Nov 16, 2016 at 07:03:38PM +0200, Tomas Winkler wrote:
> > > Functions tpm_transmit and transmit_cmd are referenced from other
> > > functions kdoc hence deserve documentation.
> > >
> > > Signed-off-by: Tomas Winkler
> >
> > Do you know how to make "make htmldocs" to generate documentation for the
> > source tree? I do not except the ones that I imported when I created an RST
> > version of the Stefans documentation.
>
> I'm using a quick script like that
> RES=tpm-kdoc.html
> rm -r $RES kdoc.err
> files=$(git ls-files drivers/char/tpm/*.[ch] includ/linux/tpm.h
> includ/linux/tpm_command.h)
>
> echo '' > $RES
> for f in ${files}; do
> ./scripts/kernel-doc -html $f >> $RES 2>> kdoc.err
> done
> echo '' >> $RES
> cat kdoc.err | grep -v 'warning: no structured comments found'
Thanks. I'll try this as soon as the release chaos is over.
/Jarkko
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH] tpm: use test_bit() to check TPM2 flag in eventlog and sysfs code
On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote: > > > On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote: > > On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote: > > > There is change done to introduce atomic bitops to set and test > > > chip->flags. > > > This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device() > > > to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag. > > > > > > Signed-off-by: Nayna Jain > > > > I'm bit lost of the purpose of this patch. > > I was using tabrm branch which has changes related to using bitops for > chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with > the existing way of checking. Replacing existing one with test_bit() check > makes it work. Same in case of tpm_sysfs_add_device(). Why didn't you just response to the thread with a review comment especially as the patch is not applied to the master branch? /Jarkko -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH RFC 2/2] tpm: refactor tpm2_get_tpm_pt to tpm2_getcap_cmd
On Tue, Nov 22, 2016 at 02:38:21PM +0530, Nayna wrote: > > > On 11/18/2016 09:43 PM, Jarkko Sakkinen wrote: > > On Fri, Nov 18, 2016 at 05:42:01PM +0530, Nayna wrote: > > > > > > > > > On 11/17/2016 11:12 PM, Jarkko Sakkinen wrote: > > > > On Thu, Nov 17, 2016 at 05:20:36PM +0530, Nayna wrote: > > > > > > > > > I tested this for capability TPM2_CAP_PCRS. It seems TPM2_CAP_PCRS > > > > > capability always returns full PCR allocation, and more_data as 0, > > > > > So, I > > > > > think the idea of looping over based on more_data may not work for > > > > > this > > > > > capability. > > > > > > > > You can always request one value at a time until there's no more. > > > > > > > > If you request N values, depending on the hardware, the hardware returns > > > > to you anything from 1 to N values. If you implement a function that > > > > requests N values in the command, you *must* handle the case where > > > > moreData is 1 even if the hardware you are testing that never happens. > > > > > > > > That's the reason why I would start with a function that you request one > > > > property of one capability and optimize it in future if it doesn't scale > > > > for some workload. > > > > > > > > Do you have a workload where it doesn't scale? > > > > > > Thanks Jarkko for explaining in detail. > > > > > > If I understood correctly, the idea is to request for one property at a > > > time, and if we need multiple properties, then to request for each of them > > > in a loop. In case of TPM2_CAP_PCRS, property is always zero. This is how > > > I > > > am calling getcap_cmd for TPM2_CAP_PCRS. > > > > > > tpm2_getcap_cmd(chip, TPM2_CAP_PCRS, 0, &cap_data, "get active pcr > > > banks"); > > > > > > Output : > > > > > > [ 17.081665] tpm: cap id to receive value is 2 > > > [ 17.081666] tpm: TPM2_CAP_COMMANDS: more data 1 > > > [ 17.081667] tpm: 2 > > > [ 17.081668] tpm: tpm2_get_active_banks ---> cap is TPM2_CAP_PCRS > > > [ 17.171665] tpm: cap id to receive value is 5 > > > [ 17.171666] tpm: TPM2_CAP_PCRS: more data 0 ---> more data is zero. > > > [ 17.171666] tpm: TPM2_CAP_PCRS: more data 0 > > > [ 17.171667] tpm: count pcr banks is 2 --> count of active pcr banks > > > information returned > > > > > > more_data is always zero here, so am not sure how to handle more_data in > > > this case ? > > > Since property_id is always zero, I am not able to request for one > > > property > > > at a time. > > > and response_buffer returns the details for both active banks. > > > > > > This is the expected behavior defined in TCG 2.0 Part 3 Commands > > > Specification (Section 30.2.1): > > > > > > "TPM_CAP_PCRS – Returns the current allocation of PCR in a > > > TPML_PCR_SELECTION. The property parameter shall be zero. The TPM will > > > always respond to this command with the full PCR allocation and moreData > > > will be NO." > > > > > > Please let me know, if I am missing something. > > > > Thanks for pointing that. I think you got it right and I had some wrong > > assumptions about 'moreData'. > > > > Here's what I propose. Do a non-generic function just for getting CAP_PCRS. > > You could call it tpm2_get_pcr_allocation() as you don't want or rather > > need to handle all the bells and whistles in that TPM command. > > > > It makes a lot more sense now than having one-size-for-all function. > > Thanks Jarkko, Yeah, Sure, I will write it as different non-generic call. > Otherwise, the function works good. > Also, I am thinking now I can write "multi-bank support for extend" on top > of master branch itself. Any issues with that ? I'm not sure what exactly I should or should not have an issue with. I can check the patch but won't apply or test it before it is used for something if that is what you are asking. /Jarkko -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH] tpm: use test_bit() to check TPM2 flag in eventlog and sysfs code
On 11/22/2016 04:29 PM, Jarkko Sakkinen wrote: > On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote: >> >> >> On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote: >>> On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote: There is change done to introduce atomic bitops to set and test chip->flags. This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device() to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag. Signed-off-by: Nayna Jain >>> >>> I'm bit lost of the purpose of this patch. >> >> I was using tabrm branch which has changes related to using bitops for >> chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with >> the existing way of checking. Replacing existing one with test_bit() check >> makes it work. Same in case of tpm_sysfs_add_device(). > > Why didn't you just response to the thread with a review comment > especially as the patch is not applied to the master branch? Oh!! Ok.. Sorry, I should have done that. Will take care next time. So basically it needs to be fixed as part of that patch. Thanks & Regards, - Nayna > > /Jarkko > -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)
On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> tpm_bios_log_teardown in order to make code flow cleaner and to enable
> to implement TPM 2.0 support later on. This is partially derived from
> the commit by Nayna Jain with the extension that also tpm1_chip_register
> is dropped.
>
> Signed-off-by: Jarkko Sakkinen
This commit remains unreviewed and tested. I'm in the author role here
so I cannot help with this. If that does not happen soon I cannot put
this into the pull request.
/Jarkko
> ---
> drivers/char/tpm/tpm-chip.c | 31 +--
> drivers/char/tpm/tpm-sysfs.c| 3 +++
> drivers/char/tpm/tpm_eventlog.c | 3 +++
> 3 files changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index d0c1872..250a651 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -276,28 +276,6 @@ static void tpm_del_char_device(struct tpm_chip *chip)
> up_write(&chip->ops_sem);
> }
>
> -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);
> -
> - rc = tpm_bios_log_setup(chip);
> -
> - return rc;
> -}
> -
> -static void tpm1_chip_unregister(struct tpm_chip *chip)
> -{
> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - return;
> -
> - tpm_bios_log_teardown(chip);
> -}
> -
> static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> {
> struct attribute **i;
> @@ -364,7 +342,9 @@ int tpm_chip_register(struct tpm_chip *chip)
> return rc;
> }
>
> - rc = tpm1_chip_register(chip);
> + tpm_sysfs_add_device(chip);
> +
> + rc = tpm_bios_log_setup(chip);
> if (rc)
> return rc;
>
> @@ -372,7 +352,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>
> rc = tpm_add_char_device(chip);
> if (rc) {
> - tpm1_chip_unregister(chip);
> + tpm_bios_log_teardown(chip);
> return rc;
> }
>
> @@ -402,8 +382,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
> void tpm_chip_unregister(struct tpm_chip *chip)
> {
> tpm_del_legacy_sysfs(chip);
> -
> - tpm1_chip_unregister(chip);
> + tpm_bios_log_teardown(chip);
> tpm_del_char_device(chip);
> }
> EXPORT_SYMBOL_GPL(tpm_chip_unregister);
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 59a1ead..848ad65 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -284,6 +284,9 @@ static const struct attribute_group tpm_dev_group = {
>
> void tpm_sysfs_add_device(struct tpm_chip *chip)
> {
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return;
> +
> /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
>* is called before ops is null'd and the sysfs core synchronizes this
>* removal so that no callbacks are running or can run again
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 62e9da6..57ac862 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -373,6 +373,9 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
> const char *name = dev_name(&chip->dev);
> unsigned int cnt;
>
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return 0;
> +
> cnt = 0;
> chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> if (is_bad(chip->bios_dir[cnt]))
> --
> 2.5.0
>
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array
On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain 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
> Signed-off-by: Nayna Jain
> Signed-off-by: Jarkko Sakkinen
This commit remains unreviewed and tested. I'm in the author role here
so I cannot help with this. If that does not happen soon I cannot put
this into the pull request.
/Jarkko
> ---
> drivers/char/tpm/tpm-chip.c | 9 ---
> drivers/char/tpm/tpm.h | 3 ++-
> drivers/char/tpm/tpm_eventlog.c | 59
> ++---
> drivers/char/tpm/tpm_eventlog.h | 10 +++
> 4 files changed, 38 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 836f056..d0c1872 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 f9401ca..9d69580 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 9467e31..62e9da6 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -368,54 +368,47 @@ 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",
> -0440, tpm_dir,
> +0440, chip->bios_dir[0],
> (void *)&tpm_binary_b_measurements_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",
> -0440, tpm_dir,
> +0440, chip->bios_dir[0],
> (void *)&tpm_ascii_b_measurements_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;
> + 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++)
> -
Re: [tpmdd-devel] [PATCH] tpm: use test_bit() to check TPM2 flag in eventlog and sysfs code
On Tue, Nov 22, 2016 at 12:59:59PM +0200, Jarkko Sakkinen wrote: > On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote: > > > > > > On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote: > > > On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote: > > > > There is change done to introduce atomic bitops to set and test > > > > chip->flags. > > > > This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device() > > > > to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag. > > > > > > > > Signed-off-by: Nayna Jain > > > > > > I'm bit lost of the purpose of this patch. > > > > I was using tabrm branch which has changes related to using bitops for > > chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with > > the existing way of checking. Replacing existing one with test_bit() check > > makes it work. Same in case of tpm_sysfs_add_device(). > > Why didn't you just response to the thread with a review comment > especially as the patch is not applied to the master branch? Anyway I updated the corresponding patches. Thanks for reporting these issues! /Jarkko -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
[tpmdd-devel] [PATCH] tpm_i2c_atmel: fix i2c_atmel_recv() when response is greater than 35 bytes
If the variable expected_len is greater than 35 bytes, i2c_atmel_recv()
ignores the amount of data already read in i2c_atmel_read_status() and
request more data than what the device is ready to supply. As result the
TPM data sent to the upper layers will miss the first 35 bytes of the
response and will be filled with garbage in the end.
TCSP_GetRandom_Internal before fix:
tpm_i2c_atmel 0-0020: i2c_atmel_send(buf=00 c1 00 00 00 0e 00 00 00 46 00 00 00
20 len=e) -> sts=14
tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=-6
tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=35
tpm_i2c_atmel 0-0020: i2c_atmel_recv reread(buf=3b ec a5 17 37 27 2a fb a0 cc
ce ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff count=1000) -> ret=46
TCSP_GetRandom_Internal after fix:
tpm_i2c_atmel 0-0020: i2c_atmel_send(buf=00 c1 00 00 00 0e 00 00 00 46 00 00 00
20 len=e) -> sts=14
tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=-6
tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=35
tpm_i2c_atmel 0-0020: i2c_atmel_recv reread(buf=00 c4 00 00 00 2e 00 00 00 00
00 00 00 20 63 dc 83 a1 55 e6 b4 5d 5a 10 70 63 28 5c 5b a8 87 ca 57 fd 45 c3
a0 62 1b c2 1d b3 d2 0d 8f 19 count=1000) -> ret=46
Signed-off-by: Fabio Urquiza
---
drivers/char/tpm/tpm_i2c_atmel.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 95ce2e9..a02f5a7 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -75,9 +75,9 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf,
size_t count)
struct tpm_output_header *hdr =
(struct tpm_output_header *)priv->buffer;
u32 expected_len;
- int rc;
+ int rc = priv->len;
- if (priv->len == 0)
+ if (rc == 0)
return -EIO;
/* Get the message size from the message header, if we didn't get the
@@ -87,7 +87,7 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf,
size_t count)
if (expected_len > count)
return -ENOMEM;
- if (priv->len >= expected_len) {
+ if (rc >= expected_len) {
dev_dbg(&chip->dev,
"%s early(buf=%*ph count=%0zx) -> ret=%d\n", __func__,
(int)min_t(size_t, 64, expected_len), buf, count,
@@ -96,7 +96,8 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf,
size_t count)
return expected_len;
}
- rc = i2c_master_recv(client, buf, expected_len);
+ memcpy(buf, priv->buffer, rc);
+ rc += i2c_master_recv(client, buf + rc, expected_len - rc);
dev_dbg(&chip->dev,
"%s reread(buf=%*ph count=%0zx) -> ret=%d\n", __func__,
(int)min_t(size_t, 64, expected_len), buf, count,
--
2.1.4
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH] tpm: use test_bit() to check TPM2 flag in eventlog and sysfs code
On Tue, Nov 22, 2016 at 04:36:56PM +0530, Nayna wrote: > > > On 11/22/2016 04:29 PM, Jarkko Sakkinen wrote: > > On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote: > > > > > > > > > On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote: > > > > On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote: > > > > > There is change done to introduce atomic bitops to set and test > > > > > chip->flags. > > > > > This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device() > > > > > to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag. > > > > > > > > > > Signed-off-by: Nayna Jain > > > > > > > > I'm bit lost of the purpose of this patch. > > > > > > I was using tabrm branch which has changes related to using bitops for > > > chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() > > > with > > > the existing way of checking. Replacing existing one with test_bit() check > > > makes it work. Same in case of tpm_sysfs_add_device(). > > > > Why didn't you just response to the thread with a review comment > > especially as the patch is not applied to the master branch? > > Oh!! Ok.. Sorry, I should have done that. > Will take care next time. > > So basically it needs to be fixed as part of that patch. NP. Thanks for reporting the issue! /Jarkko -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH 2/2] tpm: Fix error code handling after tpm_bios_log_setup
On Mon, Nov 21, 2016 at 10:29:15PM +0200, Jarkko Sakkinen wrote: > I just went through tpm_bios_log_setup(), tpm_read_log_of() and > tpm_read_log_acpi(). The error handling is sound now but the condition > should be in tpm_bios_log_setup(). Not in tpm_chip_register(). > > It is applied but if you don't mind I would like make a small commit > that moves the condition to that function. I'm not particularly concerned where the check lives.. Jason -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH 2/2] tpm: Fix error code handling after tpm_bios_log_setup
On Tue, Nov 22, 2016 at 09:37:20AM -0700, Jason Gunthorpe wrote: > On Mon, Nov 21, 2016 at 10:29:15PM +0200, Jarkko Sakkinen wrote: > > > I just went through tpm_bios_log_setup(), tpm_read_log_of() and > > tpm_read_log_acpi(). The error handling is sound now but the condition > > should be in tpm_bios_log_setup(). Not in tpm_chip_register(). > > > > It is applied but if you don't mind I would like make a small commit > > that moves the condition to that function. > > I'm not particularly concerned where the check lives.. It's just a minor glitch but still an obvious inconsistency. Maybe I'll ignore for th moment as everything is well tested. There is still two commits in the series that are untested and unreviewed. /Jarkko -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)
On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote: > On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote: > > Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and > > tpm_bios_log_teardown in order to make code flow cleaner and to enable > > to implement TPM 2.0 support later on. This is partially derived from > > the commit by Nayna Jain with the extension that also tpm1_chip_register > > is dropped. > > > > Signed-off-by: Jarkko Sakkinen > > This commit remains unreviewed and tested. I'm in the author role here > so I cannot help with this. If that does not happen soon I cannot put > this into the pull request. I tested it on my ARM system when I tested your branch. I think it looks better this way.. Reviewed-by: Jason Gunthorpe Jason -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array
On Tue, Nov 22, 2016 at 01:23:33PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain 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
> > Signed-off-by: Nayna Jain
> > Signed-off-by: Jarkko Sakkinen
>
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.
Nayna must have tested it, looks OK to me..
> > +err:
> > + chip->bios_dir[cnt] = NULL;
> > + tpm_bios_log_teardown(chip);
> > + return -EIO;
Except that return should ideally be PTR_ERR(chip->bios_dir[cnt])
.. and we still set ERR_PTR into bios_dir in the ENODEV case, so the
overall series is still broken if securityfs is compiled out.
Lets fix this all like this - which is a good enough reason to leave the
ENODEV detect alone - just squash this into your patch:
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 2a15b866ac257a..11bb1138a8282e 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -356,15 +356,6 @@ static const struct file_operations
tpm_bios_measurements_ops = {
.release = tpm_bios_measurements_release,
};
-static int is_bad(void *p)
-{
- if (!p)
- return 1;
- if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
- return 1;
- return 0;
-}
-
static int tpm_read_log(struct tpm_chip *chip)
{
int rc;
@@ -390,7 +381,8 @@ static int tpm_read_log(struct tpm_chip *chip)
* If an event log is found then the securityfs files are setup to
* export it to userspace, otherwise nothing is done.
*
- * Returns -ENODEV if the firmware has no event log.
+ * Returns -ENODEV if the firmware has no event log or securityfs is not
+ * supported.
*/
int tpm_bios_log_setup(struct tpm_chip *chip)
{
@@ -407,7 +399,10 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
cnt = 0;
chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
- if (is_bad(chip->bios_dir[cnt]))
+ /* NOTE: securityfs_create_dir can return ENODEV if securityfs is
+* compiled out. The caller should ignore the ENODEV return code.
+*/
+ if (IS_ERR(chip->bios_dir[cnt]))
goto err;
cnt++;
@@ -419,7 +414,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
0440, chip->bios_dir[0],
(void *)&chip->bin_log_seqops,
&tpm_bios_measurements_ops);
- if (is_bad(chip->bios_dir[cnt]))
+ if (IS_ERR(chip->bios_dir[cnt]))
goto err;
cnt++;
@@ -431,16 +426,17 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
0440, chip->bios_dir[0],
(void *)&chip->ascii_log_seqops,
&tpm_bios_measurements_ops);
- if (is_bad(chip->bios_dir[cnt]))
+ if (IS_ERR(chip->bios_dir[cnt]))
goto err;
cnt++;
return 0;
err:
+ rc = PTR_ERR(chip->bios_dir[cnt]);
chip->bios_dir[cnt] = NULL;
tpm_bios_log_teardown(chip);
- return -EIO;
+ return rc;
}
void tpm_bios_log_teardown(struct tpm_chip *chip)
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH] tpm_i2c_atmel: fix i2c_atmel_recv() when response is greater than 35 bytes
On Tue, Nov 22, 2016 at 09:40:07AM -0300, Fabio Urquiza wrote: > If the variable expected_len is greater than 35 bytes, i2c_atmel_recv() > ignores the amount of data already read in i2c_atmel_read_status() and > request more data than what the device is ready to supply. As result the > TPM data sent to the upper layers will miss the first 35 bytes of the > response and will be filled with garbage in the end. I'm concerned your chip is not behaving the same as mine, I have to dig out my hardware and check. I'm fairly certain I would have hit this.. Do you know the revision code for your chip? IIRC my TPM re-read the message starting from byte 0, which is why the code is like this. Jason -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH] tpm_i2c_atmel: fix i2c_atmel_recv() when response is greater than 35 bytes
On Tue, Nov 22, 2016 at 2:08 PM, Jason Gunthorpe wrote: > > On Tue, Nov 22, 2016 at 09:40:07AM -0300, Fabio Urquiza wrote: > > If the variable expected_len is greater than 35 bytes, i2c_atmel_recv() > > ignores the amount of data already read in i2c_atmel_read_status() and > > request more data than what the device is ready to supply. As result the > > TPM data sent to the upper layers will miss the first 35 bytes of the > > response and will be filled with garbage in the end. > > I'm concerned your chip is not behaving the same as mine, I have to > dig out my hardware and check. I'm fairly certain I would have hit > this.. > > Do you know the revision code for your chip? > > IIRC my TPM re-read the message starting from byte 0, which is why the > code is like this. > > Jason I'm working on an emulator for that chip and I don't have the real hardware to check. That behavior makes sense. I will change the emulator to follow that. You can ignore that patch, then. Thanks, -Fabio Urquiza -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array
On 11/22/2016 04:53 PM, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain 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
>> Signed-off-by: Nayna Jain
>> Signed-off-by: Jarkko Sakkinen
>
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.
Tested-By: Nayna Jain
Thanks & Regards,
- Nayna
>
> /Jarkko
>
>
>> ---
>> drivers/char/tpm/tpm-chip.c | 9 ---
>> drivers/char/tpm/tpm.h | 3 ++-
>> drivers/char/tpm/tpm_eventlog.c | 59
>> ++---
>> drivers/char/tpm/tpm_eventlog.h | 10 +++
>> 4 files changed, 38 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 836f056..d0c1872 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 f9401ca..9d69580 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 9467e31..62e9da6 100644
>> --- a/drivers/char/tpm/tpm_eventlog.c
>> +++ b/drivers/char/tpm/tpm_eventlog.c
>> @@ -368,54 +368,47 @@ 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",
>> - 0440, tpm_dir,
>> + 0440, chip->bios_dir[0],
>> (void *)&tpm_binary_b_measurements_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",
>> - 0440, tpm_dir,
>> + 0440, chip->bios_dir[0],
>> (void *)&tpm_ascii_b_measurements_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;
>> +tpm_bios_log_teardown(chip);
>> +return -EIO;
>> }
>>
>> -v
