From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753651AbbJGKdY (ORCPT ); Wed, 7 Oct 2015 06:33:24 -0400 Received: from mailext.sit.fraunhofer.de ([141.12.72.89]:44505 "EHLO mailext.sit.fraunhofer.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751417AbbJGKdW convert rfc822-to-8bit (ORCPT ); Wed, 7 Oct 2015 06:33:22 -0400 From: "Fuchs, Andreas" To: Jarkko Sakkinen CC: "tpmdd-devel@lists.sourceforge.net" , "linux-kernel@vger.kernel.org" , David Howells , "gregkh@linuxfoundation.org" , "open list:KEYS-TRUSTED" , "open list:KEYS-TRUSTED" , James Morris , "David Safford" , "akpm@linux-foundation.org" , "Serge E. Hallyn" , "josh@joshtripplet.org" , "richard.l.maliszewski@intel.com" , "monty.wiseman@intel.com" , "will.c.arthur@intel.com" , "artem.bityutskiy@linux.intel.com" Subject: RE: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips Thread-Topic: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips Thread-Index: AQHQ/O3SmaygPpaHSk2oQAnc2S6f+55X715zgAGBTICAAkH+poAAxDWAgAAm21GAABC6gIAAIhu2///0c4CAACRtB///5p2AgAAh/fr//+aoAAAk7+fDAAkelwAABYs90QAAAGWAAATvKcEAI5TxgAAEY5LB Date: Wed, 7 Oct 2015 10:32:41 +0000 Message-ID: <9F48E1A823B03B4790B7E6E69430724D9D7B1EEE@EXCH2010A.sit.fraunhofer.de> References: <20151005131733.GA4459@intel.com> <9F48E1A823B03B4790B7E6E69430724D9D7AEBD5@EXCH2010A.sit.fraunhofer.de> <20151005135703.GA6196@intel.com> <9F48E1A823B03B4790B7E6E69430724D9D7AEC1D@EXCH2010A.sit.fraunhofer.de> <20151005142800.GA7205@intel.com> <9F48E1A823B03B4790B7E6E69430724D9D7AF14E@EXCH2010A.sit.fraunhofer.de> <20151006122644.GA22991@intel.com> <9F48E1A823B03B4790B7E6E69430724D9D7B056A@EXCH2010A.sit.fraunhofer.de> <20151006150531.GA7075@intel.com> <9F48E1A823B03B4790B7E6E69430724D9D7B1E84@EXCH2010A.sit.fraunhofer.de>,<20151007102537.GA7261@intel.com> In-Reply-To: <20151007102537.GA7261@intel.com> Accept-Language: en-US, de-DE Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [79.242.103.12] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > > > > > 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. Fine with me. P.S. do you have a git repo with all your queued and future patches at HEAD ? Cheers, Andreas