From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> To: James Bottomley <James.Bottomley@HansenPartnership.com> Cc: linux-security-module@vger.kernel.org, tpmdd-devel@lists.sourceforge.net, open list <linux-kernel@vger.kernel.org> Subject: Re: [tpmdd-devel] [PATCH RFC v3 5/5] tpm2: expose resource manager via a device link /dev/tpms<n> Date: Tue, 24 Jan 2017 14:03:44 +0200 [thread overview] Message-ID: <20170124120344.iso5su7oqhednoeo@intel.com> (raw) In-Reply-To: <1485188095.2534.6.camel@HansenPartnership.com> On Mon, Jan 23, 2017 at 08:14:55AM -0800, James Bottomley wrote: > On Mon, 2017-01-23 at 16:09 +0200, Jarkko Sakkinen wrote: > > On Sun, Jan 22, 2017 at 01:36:28PM -0800, James Bottomley wrote: > > > On Sun, 2017-01-22 at 23:04 +0200, Jarkko Sakkinen wrote: > > > > On Sun, Jan 22, 2017 at 11:01:07PM +0200, Jarkko Sakkinen wrote: > > > > > On Sun, Jan 22, 2017 at 10:30:55PM +0200, Jarkko Sakkinen > > > > > wrote: > > > > > > On Sun, Jan 22, 2017 at 10:48:12AM -0800, James Bottomley > > > > > > wrote: > > > > > > > On Sun, 2017-01-22 at 09:49 -0800, James Bottomley wrote: > > > > > > > > On Fri, 2017-01-20 at 23:05 +0200, Jarkko Sakkinen wrote: > > > > > > > > > 'tabrm4' branch has been now rebased. It's now on top > > > > > > > > > of > > > > > > > > > master > > > > > > > > > branch that contains Stefan's latest patch (min body > > > > > > > > > length > > > > > > > > > check) > > > > > > > > > that I've reviewed and tested. It also contains your > > > > > > > > > updated > > > > > > > > > /dev/tpms patch. > > > > > > > > > > > > > > > > > > I guess the 5 commits that are there now are such that > > > > > > > > > we > > > > > > > > > have > > > > > > > > > fairly good consensus, don't we? If so, can I add your > > > > > > > > > reviewed-by > > > > > > > > > and tested-by to my commits and vice versa? > > > > > > > > > > > > > > > > We're still failing my test_transients. This is the full > > > > > > > > python of > > > > > > > > the test case: > > > > > > > > > > > > > > > > > > > > > > > > def test_transients(self): > > > > > > > > k = self.open_transients() > > > > > > > > self.c.flush_context(k[0]) > > > > > > > > self.c.change_auth(self.c.SRK, k[1], None, pwd1) > > > > > > > > ... > > > > > > > > > > > > > > > > It's failing at self.c.flush_context(k[0]) with > > > > > > > > TPM_RC_VALUE. > > > > > > > > It's > > > > > > > > the same problem Ken complained about: TPM2_FlushContext > > > > > > > > doesn't have > > > > > > > > a declared handle area so we don't translate the handle > > > > > > > > being > > > > > > > > sent > > > > > > > > down. We have to fix this either by intercepting the > > > > > > > > flush > > > > > > > > and > > > > > > > > manually translating the context, or by being dangerously > > > > > > > > clever and > > > > > > > > marking flush as a command which takes one handle. > > > > > > > > > > > > > > This is what the dangerously clever fix looks like. With > > > > > > > this > > > > > > > and a > > > > > > > few other changes, my smoke tests now pass. > > > > > > > > > > > > > > James > > > > > > > > > > > > I don't want to be clever here. I will rather intercept the > > > > > > body > > > > > > and > > > > > > try to keep the core code simple and easy to understand. > > > > > > > > > > It came out quite clean actually. > > > > > > > > > > I just encapsulated handle mapping and have this in the > > > > > beginning > > > > > of > > > > > tpm2_map_command: > > > > > > > > > > if (cc == TPM2_CC_FLUSH_CONTEXT) > > > > > return tpm2_map_to_phandle(space, > > > > > &cmd[TPM_HEADER_SIZE]); > > > > > > > > > > I think this documents better what is actually going on than > > > > > tinkering > > > > > cc_attr_tbl. > > > > > > > > > > /Jarkko > > > > > > > > Actually what you suggested is much better idea because it will > > > > also > > > > take care of validation. > > > > > > Yes, that's why it's clever ... I'm just always wary of clever code > > > because of the Kernighan principle. > > > > > > > I'm still going to keep tpm2_map_to_phandle because it makes the > > > > code flow a lot cleaner and probably sessions have to anyway make > > > > it > > > > even more complicated. > > > > > > OK, there's one more thing that seems to be causing problems: when > > > tpm2_save_context fails because the handle no longer exists (like > > > it's > > > been flushed) it returns TPM_RC_REFERENCE_H0 not TPM_RC_HANDLE (the > > > session code does seem to return TPM_RC_HANDLE under some > > > circumstances). > > > > > > James > > > > What is your way for reproducing this issue? Just want to add > > a test case for my smoke test suite so that I can verify that > > the issue is fixed once I've fixed it. > > > It's the test_handle_clearing test in tpm2_sessions_smoke.py. It's > probably easier if I publish the current state of my mods to your tpm2 > -scripts, so here they are: > > http://git.kernel.org/cgit/linux/kernel/git/jejb/tpm2-scripts.git/ Thanks. I think you can use also this list to send updates to my test scripts in future. > James /Jarkko
WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> To: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH RFC v3 5/5] tpm2: expose resource manager via a device link /dev/tpms<n> Date: Tue, 24 Jan 2017 14:03:44 +0200 [thread overview] Message-ID: <20170124120344.iso5su7oqhednoeo@intel.com> (raw) In-Reply-To: <1485188095.2534.6.camel-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> On Mon, Jan 23, 2017 at 08:14:55AM -0800, James Bottomley wrote: > On Mon, 2017-01-23 at 16:09 +0200, Jarkko Sakkinen wrote: > > On Sun, Jan 22, 2017 at 01:36:28PM -0800, James Bottomley wrote: > > > On Sun, 2017-01-22 at 23:04 +0200, Jarkko Sakkinen wrote: > > > > On Sun, Jan 22, 2017 at 11:01:07PM +0200, Jarkko Sakkinen wrote: > > > > > On Sun, Jan 22, 2017 at 10:30:55PM +0200, Jarkko Sakkinen > > > > > wrote: > > > > > > On Sun, Jan 22, 2017 at 10:48:12AM -0800, James Bottomley > > > > > > wrote: > > > > > > > On Sun, 2017-01-22 at 09:49 -0800, James Bottomley wrote: > > > > > > > > On Fri, 2017-01-20 at 23:05 +0200, Jarkko Sakkinen wrote: > > > > > > > > > 'tabrm4' branch has been now rebased. It's now on top > > > > > > > > > of > > > > > > > > > master > > > > > > > > > branch that contains Stefan's latest patch (min body > > > > > > > > > length > > > > > > > > > check) > > > > > > > > > that I've reviewed and tested. It also contains your > > > > > > > > > updated > > > > > > > > > /dev/tpms patch. > > > > > > > > > > > > > > > > > > I guess the 5 commits that are there now are such that > > > > > > > > > we > > > > > > > > > have > > > > > > > > > fairly good consensus, don't we? If so, can I add your > > > > > > > > > reviewed-by > > > > > > > > > and tested-by to my commits and vice versa? > > > > > > > > > > > > > > > > We're still failing my test_transients. This is the full > > > > > > > > python of > > > > > > > > the test case: > > > > > > > > > > > > > > > > > > > > > > > > def test_transients(self): > > > > > > > > k = self.open_transients() > > > > > > > > self.c.flush_context(k[0]) > > > > > > > > self.c.change_auth(self.c.SRK, k[1], None, pwd1) > > > > > > > > ... > > > > > > > > > > > > > > > > It's failing at self.c.flush_context(k[0]) with > > > > > > > > TPM_RC_VALUE. > > > > > > > > It's > > > > > > > > the same problem Ken complained about: TPM2_FlushContext > > > > > > > > doesn't have > > > > > > > > a declared handle area so we don't translate the handle > > > > > > > > being > > > > > > > > sent > > > > > > > > down. We have to fix this either by intercepting the > > > > > > > > flush > > > > > > > > and > > > > > > > > manually translating the context, or by being dangerously > > > > > > > > clever and > > > > > > > > marking flush as a command which takes one handle. > > > > > > > > > > > > > > This is what the dangerously clever fix looks like. With > > > > > > > this > > > > > > > and a > > > > > > > few other changes, my smoke tests now pass. > > > > > > > > > > > > > > James > > > > > > > > > > > > I don't want to be clever here. I will rather intercept the > > > > > > body > > > > > > and > > > > > > try to keep the core code simple and easy to understand. > > > > > > > > > > It came out quite clean actually. > > > > > > > > > > I just encapsulated handle mapping and have this in the > > > > > beginning > > > > > of > > > > > tpm2_map_command: > > > > > > > > > > if (cc == TPM2_CC_FLUSH_CONTEXT) > > > > > return tpm2_map_to_phandle(space, > > > > > &cmd[TPM_HEADER_SIZE]); > > > > > > > > > > I think this documents better what is actually going on than > > > > > tinkering > > > > > cc_attr_tbl. > > > > > > > > > > /Jarkko > > > > > > > > Actually what you suggested is much better idea because it will > > > > also > > > > take care of validation. > > > > > > Yes, that's why it's clever ... I'm just always wary of clever code > > > because of the Kernighan principle. > > > > > > > I'm still going to keep tpm2_map_to_phandle because it makes the > > > > code flow a lot cleaner and probably sessions have to anyway make > > > > it > > > > even more complicated. > > > > > > OK, there's one more thing that seems to be causing problems: when > > > tpm2_save_context fails because the handle no longer exists (like > > > it's > > > been flushed) it returns TPM_RC_REFERENCE_H0 not TPM_RC_HANDLE (the > > > session code does seem to return TPM_RC_HANDLE under some > > > circumstances). > > > > > > James > > > > What is your way for reproducing this issue? Just want to add > > a test case for my smoke test suite so that I can verify that > > the issue is fixed once I've fixed it. > > > It's the test_handle_clearing test in tpm2_sessions_smoke.py. It's > probably easier if I publish the current state of my mods to your tpm2 > -scripts, so here they are: > > http://git.kernel.org/cgit/linux/kernel/git/jejb/tpm2-scripts.git/ Thanks. I think you can use also this list to send updates to my test scripts in future. > James /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
next prev parent reply other threads:[~2017-01-24 12:03 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-01-16 13:12 [PATCH RFC v3 0/5] RFC: in-kernel resource manager Jarkko Sakkinen 2017-01-16 13:12 ` Jarkko Sakkinen 2017-01-16 13:12 ` [PATCH RFC v3 1/5] tpm: validate TPM 2.0 commands Jarkko Sakkinen 2017-01-16 13:12 ` Jarkko Sakkinen 2017-01-16 13:12 ` [PATCH RFC v3 2/5] tpm: export tpm2_flush_context_cmd Jarkko Sakkinen 2017-01-16 13:12 ` Jarkko Sakkinen 2017-01-16 13:12 ` [PATCH RFC v3 3/5] tpm: infrastructure for TPM spaces Jarkko Sakkinen 2017-01-16 13:12 ` Jarkko Sakkinen 2017-01-16 13:12 ` [PATCH RFC v3 4/5] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c Jarkko Sakkinen 2017-01-16 13:12 ` Jarkko Sakkinen 2017-01-16 13:12 ` [PATCH RFC v3 5/5] tpm2: expose resource manager via a device link /dev/tpms<n> Jarkko Sakkinen 2017-01-16 13:12 ` Jarkko Sakkinen 2017-01-16 16:14 ` Jason Gunthorpe 2017-01-16 17:24 ` Jarkko Sakkinen 2017-01-16 17:28 ` [tpmdd-devel] " James Bottomley 2017-01-17 7:14 ` Jarkko Sakkinen 2017-01-18 15:01 ` James Bottomley 2017-01-19 10:49 ` Jarkko Sakkinen 2017-01-19 12:19 ` James Bottomley 2017-01-20 13:39 ` Jarkko Sakkinen 2017-01-20 21:05 ` Jarkko Sakkinen 2017-01-20 21:05 ` Jarkko Sakkinen 2017-01-21 19:28 ` [tpmdd-devel] " James Bottomley 2017-01-22 14:49 ` Jarkko Sakkinen 2017-01-22 14:49 ` Jarkko Sakkinen 2017-01-21 20:38 ` [tpmdd-devel] " James Bottomley 2017-01-21 20:38 ` James Bottomley 2017-01-22 14:49 ` [tpmdd-devel] " Jarkko Sakkinen 2017-01-22 14:49 ` Jarkko Sakkinen 2017-01-22 17:49 ` [tpmdd-devel] " James Bottomley 2017-01-22 18:48 ` James Bottomley 2017-01-22 20:30 ` Jarkko Sakkinen 2017-01-22 21:01 ` Jarkko Sakkinen 2017-01-22 21:04 ` Jarkko Sakkinen 2017-01-22 21:36 ` James Bottomley 2017-01-23 14:09 ` Jarkko Sakkinen 2017-01-23 16:14 ` James Bottomley 2017-01-23 16:14 ` James Bottomley 2017-01-24 12:03 ` Jarkko Sakkinen [this message] 2017-01-24 12:03 ` Jarkko Sakkinen 2017-01-23 16:58 ` [tpmdd-devel] " Jarkko Sakkinen 2017-01-23 21:42 ` Jarkko Sakkinen 2017-01-23 21:42 ` Jarkko Sakkinen 2017-01-23 22:16 ` [tpmdd-devel] " James Bottomley 2017-01-23 22:16 ` James Bottomley 2017-01-25 13:40 ` [tpmdd-devel] " Jarkko Sakkinen 2017-01-25 13:42 ` Jarkko Sakkinen 2017-01-27 0:29 ` James Bottomley 2017-01-27 0:29 ` James Bottomley 2017-01-27 6:45 ` [tpmdd-devel] " Jarkko Sakkinen 2017-01-27 6:45 ` Jarkko Sakkinen 2017-01-25 20:23 ` [tpmdd-devel] " Jarkko Sakkinen [not found] ` <1485209797.2534.29.camel-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> 2017-01-27 22:01 ` Ken Goldman 2017-01-22 20:24 ` [tpmdd-devel] " Jarkko Sakkinen 2017-01-22 20:24 ` Jarkko Sakkinen 2017-01-19 10:42 ` Jarkko Sakkinen 2017-01-19 10:42 ` Jarkko Sakkinen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170124120344.iso5su7oqhednoeo@intel.com \ --to=jarkko.sakkinen@linux.intel.com \ --cc=James.Bottomley@HansenPartnership.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=tpmdd-devel@lists.sourceforge.net \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.