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: Sun, 22 Jan 2017 16:49:50 +0200 [thread overview] Message-ID: <20170122144950.gjm63hjnyqu72hfy@intel.com> (raw) In-Reply-To: <1485031136.26703.16.camel@HansenPartnership.com> On Sat, Jan 21, 2017 at 12:38:56PM -0800, James Bottomley wrote: > On Fri, 2017-01-20 at 23:05 +0200, Jarkko Sakkinen wrote: > > On Fri, Jan 20, 2017 at 03:39:14PM +0200, Jarkko Sakkinen wrote: > > > On Thu, Jan 19, 2017 at 07:19:40AM -0500, James Bottomley wrote: > > > > On Thu, 2017-01-19 at 12:49 +0200, Jarkko Sakkinen wrote: > > > > > On Wed, Jan 18, 2017 at 10:01:03AM -0500, James Bottomley > > > > > wrote: > > > > > > On Mon, 2017-01-16 at 15:12 +0200, Jarkko Sakkinen wrote: > > > > > > > From: James Bottomley < > > > > > > > James.Bottomley@HansenPartnership.com> > > > > > > > > > > > > > > Currently the Resource Manager (RM) is not exposed to > > > > > > > userspace. > > > > > > > Make this exposure via a separate device, which can now be > > > > > > > opened multiple times because each read/write transaction > > > > > > > goes > > > > > > > separately via the RM. > > > > > > > > > > > > > > Concurrency is protected by the chip->tpm_mutex for each > > > > > > > read/write transaction separately. The TPM is cleared of > > > > > > > all > > > > > > > transient objects by the time the mutex is dropped, so > > > > > > > there > > > > > > > should be no interference between the kernel and userspace. > > > > > > > > > > > > There's actually a missing kfree of context_buf on the > > > > > > tpms_release > > > > > > path as well. This patch fixes it up. > > > > > > > > > > Can you send me a fresh version of the whole patch so that I > > > > > can > > > > > include to v4 that includes also changes that I requested in my > > > > > recent comments + all the fixes? > > > > > > > > Sure, I think the attached is basically it > > > > > > > > James > > > > > > Thank you! > > > > '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? > > It looks like there's another problem: you need a continue after the > transient object is garbage collected otherwise the code falls through, > does a flush which fails and then adds a ~0 as the handle meaning we'll > have a mismatch between the saved contexts and the handles. > > James Oops, my bad. It's now fixed. Thank you! /Jarkko > > --- > > commit 0da3f83ce889379bd1741a11b07a30818a223924 > Author: James Bottomley <James.Bottomley@HansenPartnership.com> > Date: Sat Jan 21 12:19:06 2017 -0800 > > continue after lazy reclaim > > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > index 8713d7f..9d87537 100644 > --- a/drivers/char/tpm/tpm2-space.c > +++ b/drivers/char/tpm/tpm2-space.c > @@ -288,9 +288,10 @@ static int tpm2_save_space(struct tpm_chip *chip) > rc = tpm2_save_context(chip, space->context_tbl[i], > space->context_buf, PAGE_SIZE, > &offset); > - if (rc == -ENOENT) > + if (rc == -ENOENT) { > space->context_tbl[i] = 0; > - else if (rc) { > + continue; > + } else if (rc) { > tpm2_flush_space(chip); > return rc; > }
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: Sun, 22 Jan 2017 16:49:50 +0200 [thread overview] Message-ID: <20170122144950.gjm63hjnyqu72hfy@intel.com> (raw) In-Reply-To: <1485031136.26703.16.camel-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> On Sat, Jan 21, 2017 at 12:38:56PM -0800, James Bottomley wrote: > On Fri, 2017-01-20 at 23:05 +0200, Jarkko Sakkinen wrote: > > On Fri, Jan 20, 2017 at 03:39:14PM +0200, Jarkko Sakkinen wrote: > > > On Thu, Jan 19, 2017 at 07:19:40AM -0500, James Bottomley wrote: > > > > On Thu, 2017-01-19 at 12:49 +0200, Jarkko Sakkinen wrote: > > > > > On Wed, Jan 18, 2017 at 10:01:03AM -0500, James Bottomley > > > > > wrote: > > > > > > On Mon, 2017-01-16 at 15:12 +0200, Jarkko Sakkinen wrote: > > > > > > > From: James Bottomley < > > > > > > > James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> > > > > > > > > > > > > > > Currently the Resource Manager (RM) is not exposed to > > > > > > > userspace. > > > > > > > Make this exposure via a separate device, which can now be > > > > > > > opened multiple times because each read/write transaction > > > > > > > goes > > > > > > > separately via the RM. > > > > > > > > > > > > > > Concurrency is protected by the chip->tpm_mutex for each > > > > > > > read/write transaction separately. The TPM is cleared of > > > > > > > all > > > > > > > transient objects by the time the mutex is dropped, so > > > > > > > there > > > > > > > should be no interference between the kernel and userspace. > > > > > > > > > > > > There's actually a missing kfree of context_buf on the > > > > > > tpms_release > > > > > > path as well. This patch fixes it up. > > > > > > > > > > Can you send me a fresh version of the whole patch so that I > > > > > can > > > > > include to v4 that includes also changes that I requested in my > > > > > recent comments + all the fixes? > > > > > > > > Sure, I think the attached is basically it > > > > > > > > James > > > > > > Thank you! > > > > '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? > > It looks like there's another problem: you need a continue after the > transient object is garbage collected otherwise the code falls through, > does a flush which fails and then adds a ~0 as the handle meaning we'll > have a mismatch between the saved contexts and the handles. > > James Oops, my bad. It's now fixed. Thank you! /Jarkko > > --- > > commit 0da3f83ce889379bd1741a11b07a30818a223924 > Author: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> > Date: Sat Jan 21 12:19:06 2017 -0800 > > continue after lazy reclaim > > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > index 8713d7f..9d87537 100644 > --- a/drivers/char/tpm/tpm2-space.c > +++ b/drivers/char/tpm/tpm2-space.c > @@ -288,9 +288,10 @@ static int tpm2_save_space(struct tpm_chip *chip) > rc = tpm2_save_context(chip, space->context_tbl[i], > space->context_buf, PAGE_SIZE, > &offset); > - if (rc == -ENOENT) > + if (rc == -ENOENT) { > space->context_tbl[i] = 0; > - else if (rc) { > + continue; > + } else if (rc) { > tpm2_flush_space(chip); > return rc; > } ------------------------------------------------------------------------------ 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-22 14:50 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 ` Jarkko Sakkinen [this message] 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 ` [tpmdd-devel] " Jarkko Sakkinen 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=20170122144950.gjm63hjnyqu72hfy@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.