Re: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Wed, 2015-10-07 at 10:32 +0000, Fuchs, Andreas wrote:
> > > > > > > I looked at Patch 3/4 and it seems you default to -EPERM
> > > > > > > on TPM2_Create()-
> > > > > > > and TPM2_Load()-failures ?
> > > > > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY
> > > > > > > and return -EBUSY
> > > > > > > in those cases. Would you agree ?
> > > > > > > (P.S. I can cross-post there if that's prefered ?)
> > > > > > 
> > > > > > Have to check the return values. I posted this patch set
> > > > > > already in
> > > > > > early July. You are the first reviewer in three months for
> > > > > > this patch
> > > > > > set.
> > > > > > 
> > > > > > I think the reason was that for TPM 1.x returned -EPERM in
> > > > > > all error
> > > > > > scenarios and I didn't want to endanger behaviour of
> > > > > > command-line tools
> > > > > > such as 'keyctl'. I would keep it that way unless you can
> > > > > > guarantee that
> > > > > > command-line tools will continue work correctly if I change
> > > > > > it to
> > > > > > -EBUSY.
> > > > > > 
> > > > > > Anyway, I will recheck this part of the patch set but
> > > > > > likely are not
> > > > > > going to do any changes because I don't want to break the
> > > > > > user space.
> > > > > > 
> > > > > > I will consider revising the patch set with keyhandle
> > > > > > required as an
> > > > > > explicit option.
> > > > > 
> > > > > Hmm... Will the old keyctl work without modification with the
> > > > > 2.0 patches
> > > > > anyways ?
> > > > 
> > > > Yes it does and it should. I've been using keyctl utility to
> > > > test my
> > > > patch set.
> > > > 
> > > > > The different keyHandle values and missing default keyHandle
> > > > > will yield
> > > > > "differences" anyways, I'd say.
> > > > > IMHO, we should get it as correct as possible given that TPM
> > > > > 2.0 is still
> > > > > very young.
> > > > > 
> > > > > Is adding "additional" ReturnCodes considered ABI
> > > > > -incompatible breaking
> > > > > anyways ?
> > > > 
> > > > Yes they are if they make the user space utiltiy malfunction.
> > > 
> > > AFAICT, keyctl just perror()s. Which is what I would have hoped.
> > > So it guess it should work with -EBUSY.
> > > Example-Trace of calls for key_adding:
> > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/key
> > > utils.c#n43
> > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/key
> > > ctl.c#n379
> > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/key
> > > ctl.c#n131
> > > 
> > > Wish I could test it myself.
> > > I understand, if you don't want to test my thoughts on this.
> > > I just cannot perform the tests myself right now... :-(
> > 
> > I would submit this change as a separate patch later anyway and not
> > include it into this patch set. If it doesn't do harm it can be
> > added
> > later on. This patch set has been now in queue for three months so
> > I
> > only make modifications that are absolutely necessary.
> > 
> > Changing keyhandle as mandatory option seems like such changes.
> > This
> > doesn't.
> 
> Fine with me.
> 
> P.S. do you have a git repo with all your queued and future patches
> at HEAD ?

In separate branches:

https://github.com/jsakkine/linux-tpm2/branches

> Cheers,
> Andreas

/Jarkko