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

[Jarkko Sakkinen]

On Wed, Oct 07, 2015 at 10:04:40AM +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/keyutils.c#n43
> http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n379
> http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.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.

> Cheers,
> Andreas--

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/