Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2

2017-02-17 Thread Davide Guerri 
Hey,sorry fro the delayed reply.Jason's patch works. I had no doubt it would, because I tried a qnd patch with manually cropped sizes. I have cleaned it up a little bit, removing printks.Thanks again!D. 

NUC-fw-fix.patch
Description: Binary data
On 17 Feb 2017, at 7:01 pm, Jason Gunthorpe  wrote:I would rather review a patch with proper commit message than just pickfrom an email thread in all cases with a proper explanation what thepatch does and why it is the right way to fix the issue.Of course, but I am waiting for Davide to say the approach even works..Jason--
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

Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2

2017-02-17 Thread Sakkinen, Jarkko 
On Fri, 2017-02-17 at 09:43 -0700, Jason Gunthorpe wrote:
> On Thu, Feb 16, 2017 at 10:36:40PM +, Winkler, Tomas wrote:
> >    This is a BIOS issue + kernel ACPI platform driver refusing to
> > handle
> >    overlapping memory space. The issue here is that the ACPI
> > platform
> >    driver is not created, however this should not prevent  the
> > tpm_crb
> >    driver from working as it registers directly the ACPI device.
> > For now
> >    you can suppress the message by adding the device in
> > forbidden_id_list
> >    in drivers/acpi/acpi_platform.c
> > 
> > 
> >    I'm already testing a fixed BIOS, not sure whether and when it
> > will be
> >    available for your particular device.
> 
> Do you think we should go ahead with the patch I sent? Or should we
> tell users they need a BIOS update?
> 
> Jason

I would rather review a patch with proper commit message than just pick
from an email thread in all cases with a proper explanation what the
patch does and why it is the right way to fix the issue.

I got an email privately and identified it as the very same issue. It
was in a NUC and the author said that he had updated the BIOS to the
latest version. That supports the view of doing some kind of workaround
to the driver, perhaps the way you did it.

/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

Re: [tpmdd-devel] tpmdd-devel Digest, Vol 110, Issue 19

2017-02-17 Thread Benoit HOUYERE 
Hi Jarko,

For your information : 

   1. [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to
  MAX_SPI_FRAMESIZE bytes (Peter Huewe)  
Tested-by: Benoit Houyere 
   2. [PATCH 0/5] Fix whole native SPI TPM driver (Peter Huewe)
Tested-by: Benoit Houyere 
   3. [PATCH 1/5] tpm_tis_spi: Use single function to   transfer data
  (Peter Huewe)
Tested-by: Benoit Houyere 
   4. [PATCH 2/5] tpm_tis_spi: Abort transfer when too many wait
  states are signaled (Peter Huewe)
Tested-by: Benoit Houyere 
   5. [PATCH 3/5] tpm_tis_spi: Check correct byte for wait  state
  indicator (Peter Huewe)
Tested-by: Benoit Houyere 
   6. [PATCH 5/5] tpm_tis_spi: Add small delay after last   transfer
  (Peter Huewe)
Tested-by: Benoit Houyere 


Best Regards,

Benoit
-Original Message-
From: tpmdd-devel-requ...@lists.sourceforge.net 
[mailto:tpmdd-devel-requ...@lists.sourceforge.net] 
Sent: jeudi 16 février 2017 17:11
To: tpmdd-devel@lists.sourceforge.net
Subject: tpmdd-devel Digest, Vol 110, Issue 19

Send tpmdd-devel mailing list submissions to
tpmdd-devel@lists.sourceforge.net

To subscribe or unsubscribe via the World Wide Web, visit
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
or, via email, send a message with subject or body 'help' to
tpmdd-devel-requ...@lists.sourceforge.net

You can reach the person managing the list at
tpmdd-devel-ow...@lists.sourceforge.net

When replying, please edit your Subject line so it is more specific than "Re: 
Contents of tpmdd-devel digest..."


Today's Topics:

   1. [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to
  MAX_SPI_FRAMESIZE bytes (Peter Huewe) 
   2. [PATCH 0/5] Fix whole native SPI TPM driver (Peter Huewe)
   3. [PATCH 1/5] tpm_tis_spi: Use single function to   transfer data
  (Peter Huewe)
   4. [PATCH 2/5] tpm_tis_spi: Abort transfer when too many wait
  states are signaled (Peter Huewe)
   5. [PATCH 3/5] tpm_tis_spi: Check correct byte for wait  state
  indicator (Peter Huewe)
   6. [PATCH 5/5] tpm_tis_spi: Add small delay after last   transfer
  (Peter Huewe)


--

Message: 1
Date: Thu, 16 Feb 2017 16:08:25 +
From: Peter Huewe 
Subject: [tpmdd-devel] [PATCH 4/5] tpm_tis_spi: Remove limitation of
transfers to MAX_SPI_FRAMESIZE bytes
To: Jarkko Sakkinen 
Cc: linux-ker...@vger.kernel.org, sta...@vger.kernel.org,
tpmdd-devel@lists.sourceforge.net,  Christophe Ricard
, Peter Huewe 
,
Alexander
Message-ID:
<1487261306-2494-5-git-send-email-peter.hu...@infineon.com>
Content-Type: text/plain

Limiting transfers to MAX_SPI_FRAMESIZE was not expected by the upper layers, 
as tpm_tis has no such limitation. Add a loop to hide that limitation.

Cc: 
Fixes: 0edbfea537d1 ("tpm/tpm_tis_spi: Add support for spi phy")
Signed-off-by: Alexander Steffen 
Signed-off-by: Peter Huewe 
---
 drivers/char/tpm/tpm_tis_spi.c | 108 ++---
 1 file changed, 57 insertions(+), 51 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c 
index 16938e2253d2..b50c5b072df3 100644
--- a/drivers/char/tpm/tpm_tis_spi.c
+++ b/drivers/char/tpm/tpm_tis_spi.c
@@ -61,68 +61,74 @@ static int tpm_tis_spi_transfer(struct tpm_tis_data *data, 
u32 addr, u8 len,  {
struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
int ret;
-   struct spi_message m;
-   struct spi_transfer spi_xfer = {
-   .tx_buf = phy->tx_buf,
-   .rx_buf = phy->rx_buf,
-   .len = 4,
-   .cs_change = 1,
-   };
-
-   if (len > MAX_SPI_FRAMESIZE)
-   return -ENOMEM;
 
-   phy->tx_buf[0] = direction | (len - 1);
-   phy->tx_buf[1] = 0xd4;
-   phy->tx_buf[2] = addr >> 8;
-   phy->tx_buf[3] = addr;
+   spi_bus_lock(phy->spi_device->master);
 
-   spi_message_init();
-   spi_message_add_tail(_xfer, );
+   while (len) {
+   struct spi_message m;
+   struct spi_transfer spi_xfer = {
+   .tx_buf = phy->tx_buf,
+   .rx_buf = phy->rx_buf,
+   .len = 4,
+   .cs_change = 1,
+   };
+   u8 transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
+
+   phy->tx_buf[0] = direction | (transfer_len - 1);
+   phy->tx_buf[1] = 0xd4;
+   phy->tx_buf[2] = addr >> 8;
+   phy->tx_buf[3] = addr;

Re: [tpmdd-devel] [PATCH] tpm: Fix expected number of response bytes of TPM1.2 PCR Extend

2017-02-17 Thread Mimi Zohar 
On Fri, 2017-02-17 at 20:45 +0200, Jarkko Sakkinen wrote:
> On Fri, Feb 17, 2017 at 07:46:38AM -0500, Mimi Zohar wrote:
> > Hi James,
> > 
> > On Wed, 2017-02-15 at 20:09 +0200, Jarkko Sakkinen wrote:
> > > On Wed, Feb 15, 2017 at 11:56:23AM -0500, Stefan Berger wrote:
> > > > The TPM1.2 PCR Extend operation only returns 20 bytes in the body,
> > > > which is the size of the PCR state.
> > > > 
> > > > This fixes a problem where IMA gets errors with every PCR Extend.
> > > > 
> > > > Fixes: c659af78eb7b ("tpm: Check size of response before accessing 
> > > > data")
> > > > Signed-off-by: Stefan Berger 
> > > > Acked-by: Mimi Zohar 
> > > 
> > > Reviewed-by: Jarkko Sakkinen 
> > 
> > This patch needs to be included with the rest of the patches being
> > upstreamed in the next open window.   Should Jarkko or I send you a pull
> > request for it?

> I'm sending a pull request after the weekend. It will contain
> only a few small scoped fixes so wouldn't it be easiest if I
> just include this to the pack?

As long as it makes it into the James' pull request to Linus, that's
fine.

Mimi


--
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

Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2

2017-02-17 Thread Jason Gunthorpe 
> I would rather review a patch with proper commit message than just pick
> from an email thread in all cases with a proper explanation what the
> patch does and why it is the right way to fix the issue.

Of course, but I am waiting for Davide to say the approach even works
..

Jason

--
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

Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2

2017-02-17 Thread Jason Gunthorpe 
On Thu, Feb 16, 2017 at 10:36:40PM +, Winkler, Tomas wrote:
>This is a BIOS issue + kernel ACPI platform driver refusing to handle
>overlapping memory space. The issue here is that the ACPI platform
>driver is not created, however this should not prevent  the tpm_crb
>driver from working as it registers directly the ACPI device. For now
>you can suppress the message by adding the device in forbidden_id_list
>in drivers/acpi/acpi_platform.c
> 
>
>I'm already testing a fixed BIOS, not sure whether and when it will be
>available for your particular device.

Do you think we should go ahead with the patch I sent? Or should we
tell users they need a BIOS update?

Jason

--
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

Re: [tpmdd-devel] [PATCH] tpm: declare tpm2_get_pcr_allocation() as static

2017-02-17 Thread Jarkko Sakkinen 
On Wed, Feb 15, 2017 at 08:02:28PM +0200, Jarkko Sakkinen wrote:
> There's no need to export tpm2_get_pcr_alloation() because it is only
> a helper function for tpm2_auto_startup(). For the same reason it does
> not make much sense to maintain documentation for it.
> 
> Signed-off-by: Jarkko Sakkinen 

Nayna, does this look good to you?

/Jarkko

> ---
>  drivers/char/tpm/tpm.h  |  1 -
>  drivers/char/tpm/tpm2-cmd.c | 94 
> ++---
>  2 files changed, 45 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 6b4e7aa..4937b56 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -554,5 +554,4 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>  unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>  int tpm2_probe(struct tpm_chip *chip);
> -ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>  #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 10f97e6..881aea9 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -997,61 +997,13 @@ int tpm2_probe(struct tpm_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(tpm2_probe);
>  
> -/**
> - * tpm2_auto_startup - Perform the standard automatic TPM initialization
> - * sequence
> - * @chip: TPM chip to use
> - *
> - * Returns 0 on success, < 0 in case of fatal error.
> - */
> -int tpm2_auto_startup(struct tpm_chip *chip)
> -{
> - int rc;
> -
> - rc = tpm_get_timeouts(chip);
> - if (rc)
> - goto out;
> -
> - rc = tpm2_do_selftest(chip);
> - if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
> - dev_err(>dev, "TPM self test failed\n");
> - goto out;
> - }
> -
> - if (rc == TPM2_RC_INITIALIZE) {
> - rc = tpm2_startup(chip, TPM2_SU_CLEAR);
> - if (rc)
> - goto out;
> -
> - rc = tpm2_do_selftest(chip);
> - if (rc) {
> - dev_err(>dev, "TPM self test failed\n");
> - goto out;
> - }
> - }
> -
> - rc = tpm2_get_pcr_allocation(chip);
> -
> -out:
> - if (rc > 0)
> - rc = -ENODEV;
> - return rc;
> -}
> -
>  struct tpm2_pcr_selection {
>   __be16  hash_alg;
>   u8  size_of_select;
>   u8  pcr_select[3];
>  } __packed;
>  
> -/**
> - * tpm2_get_pcr_allocation() - get TPM active PCR banks.
> - *
> - * @chip: TPM chip to use.
> - *
> - * Return: Same as with tpm_transmit_cmd.
> - */
> -ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> +static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  {
>   struct tpm2_pcr_selection pcr_selection;
>   struct tpm_buf buf;
> @@ -1114,3 +1066,47 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  
>   return rc;
>  }
> +
> +/**
> + * tpm2_auto_startup - Perform the standard automatic TPM initialization
> + * sequence
> + * @chip: TPM chip to use
> + *
> + * Initializes timeout values for operation and command durations, conducts
> + * a self-test and reads the list of active PCR banks.
> + *
> + * Return: 0 on success. Otherwise, a system error code is returned.
> + */
> +int tpm2_auto_startup(struct tpm_chip *chip)
> +{
> + int rc;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto out;
> +
> + rc = tpm2_do_selftest(chip);
> + if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
> + dev_err(>dev, "TPM self test failed\n");
> + goto out;
> + }
> +
> + if (rc == TPM2_RC_INITIALIZE) {
> + rc = tpm2_startup(chip, TPM2_SU_CLEAR);
> + if (rc)
> + goto out;
> +
> + rc = tpm2_do_selftest(chip);
> + if (rc) {
> + dev_err(>dev, "TPM self test failed\n");
> + goto out;
> + }
> + }
> +
> + rc = tpm2_get_pcr_allocation(chip);
> +
> +out:
> + if (rc > 0)
> + rc = -ENODEV;
> + return rc;
> +}
> -- 
> 2.9.3
> 
> --
> 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

--
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

Re: [tpmdd-devel] [RFC] tpm2-space: add handling for global session exhaustion

2017-02-17 Thread Jarkko Sakkinen 
On Fri, Feb 17, 2017 at 03:56:26AM -0600, Dr. Greg Wettstein wrote:
> On Thu, Feb 16, 2017 at 10:33:04PM +0200, Jarkko Sakkinen wrote:
> 
> Good morning to everyone.
> 
> > On Thu, Feb 16, 2017 at 02:06:42PM -0600, Dr. Greg Wettstein wrote:
> > > Just as an aside, has anyone given any thought about TPM2 resource
> > > management in things like TXT/tboot environments?  The current tboot
> > > code makes a rather naive assumption that it can take a handle slot to
> > > protect its platform verification secret.  Doing resource management
> > > correctly will require addressing extra-OS environments such as this
> > > which may have TPM2 state requirement issues.
> 
> > The current implementation handles stuff created from regular
> > /dev/tpm0 so I do not think this would be an issue. You can only
> > access objects from a TPM space that are created within that space.
> 
> Unless I misunderstand the number of transient objects which can be
> managed is a characteristic of the hardware and is a limited resource,
> hence our discussion on the notion of a resource manager to shuttle
> context in and out of these limited slots.
> 
> On a Kabylake system, running the following command:
> 
> getcapability -cap 6 | grep trans
> 
> After booting into a TXT mediated measured launch environment (MLE) yields
> the following:
> 
> TPM_PT 010e value 0003 TPM_PT_HR_TRANSIENT_MIN - the minimum number 
> of transient objects that can be held in TPM RAM
> 
> TPM_PT 0207 value 0002 TPM_PT_HR_TRANSIENT_AVAIL - estimate of the 
> number of additional transient objects that could be loaded into TPM RAM
> 
> Booting without TXT results in the getcapability call indicating that
> three slots are available.  Based on that and reading the tboot code,
> we are assuming the occupied slot is the ephemeral primary key
> generated by tboot which seals the verification secret.
> 
> In an MLE it is possible to create and then flush a new ephemeral
> primary key which results in the following getcapability output:
> 
> TPM_PT 0207 value 0003 TPM_PT_HR_TRANSIENT_AVAIL - estimate of
> the number of additional transient objects that could be loaded into TPM RAM
> 
> Which is probably going to be pretty surprising to tboot in the event
> that it tries to re-verify the system state after a suspend event.
> 
> So based on that it would seem there would need to be some semblance
> of cooperation between the resource manager and an extra-OS
> utilization of TPM2 resources such as tboot.
> 
> Thoughts?

The driver swaps in and out all the objects for one send-receive cycle.
So unless the driver is sending a command to a TPM the resource manager
occupies zero slots. I do not see reason for forseeable future to change
this pattern.

I discussed about some "lazier" schemes for swapping with James an Ken
in the early Fall but came into conclusion that it would make the RM
really complicated. There would have to be something show stopper work
load to even to start consider it.

With the capacity of current TPMs and amount of traffic and workloads
it is really not a worth of the trouble.

I guess the way we do swapping kind of indirectly sorts out the issue
you described, doesn't it?

/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

Re: [tpmdd-devel] [PATCH] tpm: Fix expected number of response bytes of TPM1.2 PCR Extend

2017-02-17 Thread Mimi Zohar 
Hi James,

On Wed, 2017-02-15 at 20:09 +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 15, 2017 at 11:56:23AM -0500, Stefan Berger wrote:
> > The TPM1.2 PCR Extend operation only returns 20 bytes in the body,
> > which is the size of the PCR state.
> > 
> > This fixes a problem where IMA gets errors with every PCR Extend.
> > 
> > Fixes: c659af78eb7b ("tpm: Check size of response before accessing data")
> > Signed-off-by: Stefan Berger 
> > Acked-by: Mimi Zohar 
> 
> Reviewed-by: Jarkko Sakkinen 

This patch needs to be included with the rest of the patches being
upstreamed in the next open window.   Should Jarkko or I send you a pull
request for it?

thanks,

Mimi


--
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