[tpmdd-devel] TPM resource manager - persistent objects should be global

Submitted by James Bottomley on March 29, 2017, 6:40 p.m.

Details

Message ID 1490812802.5647.4.camel@linux.vnet.ibm.com
State New
Headers show

Commit Message

James Bottomley March 29, 2017, 6:40 p.m.
On Tue, 2017-03-28 at 17:39 -0400, Ken Goldman wrote:
> I have a persistent object at 81000001.
> 
> getcapability through /dev/tpm0 shows it.  The same command through 
> /dev/tpmrm0 does not.  This causes further problems in the
> application.
> 
> While transient objects are per connection, IMHO persistent objects 
> should be global.
> 
> ~~
> 
> Warning:  I think I'm using the latest TPM device driver from
> 
> git://git.infradead.org/users/jjs/linux-tpmdd.git
> 
> but I'm new to both git and kernel building, so I could be wrong.  If
> you think the above should work, it could be my error.

It should work.  It turns out the body mapping code is overzealous and
errors out when it should just pass through.  The same thing happens
with the PCRs as well.

This should fix it for both.

James

---



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Comments

Jarkko Sakkinen March 31, 2017, 8:14 a.m.
Ken, James, thank you. I'll squash this before PR.

/Jarkko

On Wed, Mar 29, 2017 at 02:40:02PM -0400, James Bottomley wrote:
> On Tue, 2017-03-28 at 17:39 -0400, Ken Goldman wrote:
> > I have a persistent object at 81000001.
> > 
> > getcapability through /dev/tpm0 shows it.  The same command through 
> > /dev/tpmrm0 does not.  This causes further problems in the
> > application.
> > 
> > While transient objects are per connection, IMHO persistent objects 
> > should be global.
> > 
> > ~~
> > 
> > Warning:  I think I'm using the latest TPM device driver from
> > 
> > git://git.infradead.org/users/jjs/linux-tpmdd.git
> > 
> > but I'm new to both git and kernel building, so I could be wrong.  If
> > you think the above should work, it could be my error.
> 
> It should work.  It turns out the body mapping code is overzealous and
> errors out when it should just pass through.  The same thing happens
> with the PCRs as well.
> 
> This should fix it for both.
> 
> James
> 
> ---
> 
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 009934269514..e2e059d8ffec 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -429,15 +429,11 @@ static int tpm2_map_response_body(struct tpm_chip *chip, u32 cc, u8 *rsp,
>  			data->handles[j] = cpu_to_be32(vhandle);
>  			j++;
>  			break;
> -		case TPM2_HT_HMAC_SESSION:
> -		case TPM2_HT_POLICY_SESSION:
> +
> +		default:
>  			data->handles[j] = cpu_to_be32(phandle);
>  			j++;
>  			break;
> -		default:
> -			dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
> -				__func__, phandle);
> -			break;
>  		}
>  
>  	}
> 
> 
> ------------------------------------------------------------------------------
> 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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Ken Goldman March 31, 2017, 2:57 p.m.
This patch worked.  Can you merge it with Jarkko's latest?

I can report that my attestation client works unmodified with the 
resource manager.  I just had to point it to /dev/tpmrm0.  It's a 
Nuvoton TPM, I2C bus, POWER architecture.

It does quite a bit - transient and persistent objects, getcapability,
reading NV indexes, policy sessions.  I did the RSA and ECC variants in 
parallel, so there should have been a bit of swapping.

On 3/29/2017 2:40 PM, James Bottomley wrote:
> On Tue, 2017-03-28 at 17:39 -0400, Ken Goldman wrote:
>> I have a persistent object at 81000001.
>>
>> getcapability through /dev/tpm0 shows it.  The same command through
>> /dev/tpmrm0 does not.  This causes further problems in the
>> application.
>>
>> While transient objects are per connection, IMHO persistent objects
>> should be global.
>>
>> ~~
>>
>> Warning:  I think I'm using the latest TPM device driver from
>>
>> git://git.infradead.org/users/jjs/linux-tpmdd.git
>>
>> but I'm new to both git and kernel building, so I could be wrong.  If
>> you think the above should work, it could be my error.
>
> It should work.  It turns out the body mapping code is overzealous and
> errors out when it should just pass through.  The same thing happens
> with the PCRs as well.
>
> This should fix it for both.
>
> James
>
> ---
>
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 009934269514..e2e059d8ffec 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -429,15 +429,11 @@ static int tpm2_map_response_body(struct tpm_chip *chip, u32 cc, u8 *rsp,
>  			data->handles[j] = cpu_to_be32(vhandle);
>  			j++;
>  			break;
> -		case TPM2_HT_HMAC_SESSION:
> -		case TPM2_HT_POLICY_SESSION:
> +
> +		default:
>  			data->handles[j] = cpu_to_be32(phandle);
>  			j++;
>  			break;
> -		default:
> -			dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
> -				__func__, phandle);
> -			break;
>  		}
>
>  	}
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Jarkko Sakkinen March 31, 2017, 6:28 p.m.
The late fixes (2) are pushed to my master branch.

/Jarkko

On Fri, Mar 31, 2017 at 10:57:15AM -0400, Ken Goldman wrote:
> This patch worked.  Can you merge it with Jarkko's latest?
> 
> I can report that my attestation client works unmodified with the 
> resource manager.  I just had to point it to /dev/tpmrm0.  It's a 
> Nuvoton TPM, I2C bus, POWER architecture.
> 
> It does quite a bit - transient and persistent objects, getcapability,
> reading NV indexes, policy sessions.  I did the RSA and ECC variants in 
> parallel, so there should have been a bit of swapping.
> 
> On 3/29/2017 2:40 PM, James Bottomley wrote:
> > On Tue, 2017-03-28 at 17:39 -0400, Ken Goldman wrote:
> >> I have a persistent object at 81000001.
> >>
> >> getcapability through /dev/tpm0 shows it.  The same command through
> >> /dev/tpmrm0 does not.  This causes further problems in the
> >> application.
> >>
> >> While transient objects are per connection, IMHO persistent objects
> >> should be global.
> >>
> >> ~~
> >>
> >> Warning:  I think I'm using the latest TPM device driver from
> >>
> >> git://git.infradead.org/users/jjs/linux-tpmdd.git
> >>
> >> but I'm new to both git and kernel building, so I could be wrong.  If
> >> you think the above should work, it could be my error.
> >
> > It should work.  It turns out the body mapping code is overzealous and
> > errors out when it should just pass through.  The same thing happens
> > with the PCRs as well.
> >
> > This should fix it for both.
> >
> > James
> >
> > ---
> >
> > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> > index 009934269514..e2e059d8ffec 100644
> > --- a/drivers/char/tpm/tpm2-space.c
> > +++ b/drivers/char/tpm/tpm2-space.c
> > @@ -429,15 +429,11 @@ static int tpm2_map_response_body(struct tpm_chip *chip, u32 cc, u8 *rsp,
> >  			data->handles[j] = cpu_to_be32(vhandle);
> >  			j++;
> >  			break;
> > -		case TPM2_HT_HMAC_SESSION:
> > -		case TPM2_HT_POLICY_SESSION:
> > +
> > +		default:
> >  			data->handles[j] = cpu_to_be32(phandle);
> >  			j++;
> >  			break;
> > -		default:
> > -			dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
> > -				__func__, phandle);
> > -			break;
> >  		}
> >
> >  	}
> >
> 
> 
> ------------------------------------------------------------------------------
> 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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch hide | download patch | download mbox

diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 009934269514..e2e059d8ffec 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -429,15 +429,11 @@  static int tpm2_map_response_body(struct tpm_chip *chip, u32 cc, u8 *rsp,
 			data->handles[j] = cpu_to_be32(vhandle);
 			j++;
 			break;
-		case TPM2_HT_HMAC_SESSION:
-		case TPM2_HT_POLICY_SESSION:
+
+		default:
 			data->handles[j] = cpu_to_be32(phandle);
 			j++;
 			break;
-		default:
-			dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
-				__func__, phandle);
-			break;
 		}
 
 	}