[tpmdd-devel] [PATCH 2/2] tpm: Fix error code handling after tpm_bios_log_setup

[Stefan Berger]

tpm_bios_log_setup() may return -ENODEV in case no log was
found. In this case we do not need to fail the device.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 3f27753..2d6530b 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -346,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip)
tpm_sysfs_add_device(chip);
 
rc = tpm_bios_log_setup(chip);
-   if (rc == -ENODEV)
+   if (rc != -ENODEV)
return rc;
 
tpm_add_ppi(chip);
-- 
2.4.3


--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH] tpm: vtpm_proxy: Do not access host's event log

[Jason Gunthorpe]

On Thu, Nov 17, 2016 at 01:25:54PM -0500, Stefan Berger wrote:
> 
> In the case of x86, tpm_read_log_of() is a stub return -ENODEV, which in
> turn fails the whole device:

Somehow this got screwed up during the lengthy review. ENODEV is the
right return from the leaf routines but the tests in tpm_eventlog di
not get fixed:

> http://git.infradead.org/users/jjs/linux-tpmdd.git/blob/4d388433e85f8257f5a9344a7acf6f499ba2b29e:/drivers/char/tpm/tpm_eventlog.h#l87

Is wrong, should be:

if (rc != -ENODEV)
   return rc;

And the one in tpm_bios_log_setup should be

if (rc != 0 && rc != -ENODEV)
return rc;

> I think the OF log reading code will also need to check for chip->dev.parent
> being NULL.

Currect! Lets get that fixed too. :(

> Further, I had the impression that the error unwinding following -ENODEV has
> an issue related to sysfs.

I don't follow this comment..

Jason

--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH] tpm: vtpm_proxy: Do not access host's event log

[Jason Gunthorpe]

1;2802;0cOn Thu, Nov 17, 2016 at 07:35:05AM -0500, Stefan Berger wrote:
> I ran the vtpm driver test suite (with -j32) a few times at that patch and
> it didn't crash. It crashes severely with later patches applied. Here's the
> current experimental patch that fixes these problems:

I can't see how setting owner has any bearing on this.. I also don't
see why it should ever fail at all... It would be great to get a root
cause here - could it be memory corruption

Getting a really bad feeling from this  :(

> iff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
> index 0cb43ef..a73295a 100644
> +++ b/drivers/char/tpm/tpm_acpi.c
> @@ -56,6 +56,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> 
>  log = >log;
> 
> +if (!chip->acpi_dev_handle)
> +return 0;
> +
> 
> // So ACPI is not supported on this device, but ACPI support is compiled in.
> I am returning 0 here, assuming it's not an OF device and the corresponding
> OF function need not be called (see below).

Return -ENODEV

> +if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
> +rc = tpm_read_log_of(chip);
> 
> // I am not sure how to handle this case, in case we get here, which would
> only be on an OF device (following 'return 0;' above), but we don't want to
> attempt to read the log there, either. I think the most straight-forward way
> would be to gate this whole function with a flag that only the vtpm proxy
> driver has: TPM_CHIP_FLAG_NO_FIRMWARE_LOG.

OF is already fine, it checks chip->dev.parent->of_node so it will
exit properly for vtpm, no need for this.

Jason

--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH] tpm: drop chip->is_open and chip->duration_adjusted

[Jarkko Sakkinen]

On Thu, Nov 17, 2016 at 04:10:29PM +0530, Nayna wrote:
> On 11/15/2016 05:14 AM, Jarkko Sakkinen wrote:
> > -   if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> > -   return 0;
> > +   if (test_bit(TPM_CHIP_FLAG_TPM2, >flags) ||
> > +   test_bit(TPM_CHIP_FLAG_VIRTUAL, >flags))
> > +   return;
> 
> guess should be "return 0;"

Ouch. Good catch. Thank you.

/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

[Nayna]

On 11/12/2016 05:32 AM, Jarkko Sakkinen wrote:
> On Fri, Nov 11, 2016 at 09:51:45AM +0530, Nayna wrote:
>>
>>
>> On 10/09/2016 03:44 PM, Jarkko Sakkinen wrote:
>>> Refactored tpm2_get_tpm_pt to tpm2_getcap_cmd, which means that it also
>>> takes capability ID as input. This is required to access
>>> TPM_CAP_HANDLES, which contains metadata needed for swapping transient
>>> data.
>>>
>>> Signed-off-by: Jarkko Sakkinen 
>>> ---
>>>drivers/char/tpm/tpm.h  |  6 +++-
>>>drivers/char/tpm/tpm2-cmd.c | 64 
>>> -
>>>drivers/char/tpm/tpm_tis_core.c |  3 +-
>>>3 files changed, 38 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>> index 0fab6d5..8176f42 100644
>>> --- a/drivers/char/tpm/tpm.h
>>> +++ b/drivers/char/tpm/tpm.h
>>> @@ -85,6 +85,10 @@ enum tpm2_capabilities {
>>> TPM2_CAP_TPM_PROPERTIES = 6,
>>>};
>>>
>>> +enum tpm2_properties {
>>> +   TPM2_PT_FAMILY_INDICATOR= 0x100,
>>> +};
>>> +
>>>enum tpm2_startup_types {
>>> TPM2_SU_CLEAR   = 0x,
>>> TPM2_SU_STATE   = 0x0001,
>>> @@ -485,7 +489,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>>>int tpm2_unseal_trusted(struct tpm_chip *chip,
>>> struct trusted_key_payload *payload,
>>> struct trusted_key_options *options);
>>> -ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
>>> +ssize_t tpm2_getcap_cmd(struct tpm_chip *chip, u32 cap_id, u32 property_id,
>>> u32 *value, const char *desc);
>>>
>>>int tpm2_auto_startup(struct tpm_chip *chip);
>>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>>> index 2900e18..fcf3d86 100644
>>> --- a/drivers/char/tpm/tpm2-cmd.c
>>> +++ b/drivers/char/tpm/tpm2-cmd.c
>>> @@ -111,13 +111,13 @@ struct tpm2_pcr_extend_in {
>>> u8  digest[TPM_DIGEST_SIZE];
>>>} __packed;
>>>
>>> -struct tpm2_get_tpm_pt_in {
>>> +struct tpm2_getcap_in {
>>> __be32  cap_id;
>>> __be32  property_id;
>>> __be32  property_cnt;
>>>} __packed;
>>>
>>> -struct tpm2_get_tpm_pt_out {
>>> +struct tpm2_getcap_out {
>>> u8  more_data;
>>> __be32  subcap_id;
>>> __be32  property_cnt;
>>> @@ -140,8 +140,8 @@ union tpm2_cmd_params {
>>> struct  tpm2_pcr_read_inpcrread_in;
>>> struct  tpm2_pcr_read_out   pcrread_out;
>>> struct  tpm2_pcr_extend_in  pcrextend_in;
>>> -   struct  tpm2_get_tpm_pt_in  get_tpm_pt_in;
>>> -   struct  tpm2_get_tpm_pt_out get_tpm_pt_out;
>>> +   struct  tpm2_getcap_in  getcap_in;
>>> +   struct  tpm2_getcap_out getcap_out;
>>> struct  tpm2_get_random_in  getrandom_in;
>>> struct  tpm2_get_random_out getrandom_out;
>>>};
>>> @@ -435,16 +435,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, 
>>> size_t max)
>>> return total ? total : -EIO;
>>>}
>>>
>>> -#define TPM2_GET_TPM_PT_IN_SIZE \
>>> -   (sizeof(struct tpm_input_header) + \
>>> -sizeof(struct tpm2_get_tpm_pt_in))
>>> -
>>> -static const struct tpm_input_header tpm2_get_tpm_pt_header = {
>>> -   .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
>>> -   .length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
>>> -   .ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
>>> -};
>>> -
>>>/**
>>> * Append TPMS_AUTH_COMMAND to the buffer. The buffer must be allocated 
>>> with
>>> * tpm_buf_alloc().
>>> @@ -750,35 +740,43 @@ out:
>>> return rc;
>>>}
>>>
>>> +#define TPM2_GETCAP_IN_SIZE \
>>> +   (sizeof(struct tpm_input_header) + sizeof(struct tpm2_getcap_in))
>>> +
>>> +static const struct tpm_input_header tpm2_getcap_header = {
>>> +   .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
>>> +   .length = cpu_to_be32(TPM2_GETCAP_IN_SIZE),
>>> +   .ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
>>> +};
>>> +
>>>/**
>>> - * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
>>> - * @chip:  TPM chip to use.
>>> - * @property_id:   property ID.
>>> - * @value: output variable.
>>> + * tpm2_getcap_cmd() - execute a TPM2_GetCapability command
>>> + * @chip:  TPM chip to use
>>> + * @cap_id:capability ID
>>> + * @property_id:   property ID
>>> + * @value: value of the property
>>> * @desc:passed to tpm_transmit_cmd()
>>> *
>>> - * 0 is returned when the operation is successful. If a negative number is
>>> - * returned it remarks a POSIX error code. If a positive number is returned
>>> - * it remarks a TPM error.
>>> + * Return: same as with tpm_transmit_cmd
>>> */
>>> -ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 
>>> *value,
>>> -   const char *desc)
>>> +ssize_t tpm2_getcap_cmd(struct tpm_chip *chip, u32 cap_id, u32 property_id,
>>> +   u32 *value, const char *desc)
>>
>> This function currently returns single