From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Peter Huewe <peterhuewe@gmx.de>, linux-security-module@vger.kernel.org, stable@vger.kernel.org, Marcel Selhorst <tpmdd@selhorst.net>, "moderated list:TPM DEVICE DRIVER" <tpmdd-devel@lists.sourceforge.net>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] tpm: fix a race condition tpm2_unseal_trusted() Date: Thu, 25 Aug 2016 17:06:10 -0400 [thread overview] Message-ID: <20160825210437.GA8658@intel.com> (raw) In-Reply-To: <20160825183059.GB1142@obsidianresearch.com> On Thu, Aug 25, 2016 at 12:30:59PM -0600, Jason Gunthorpe wrote: > On Tue, Aug 23, 2016 at 08:57:22PM -0400, Jarkko Sakkinen wrote: > > > + if (flags & TPM_TRANSMIT_LOCK) > > + mutex_lock(&chip->tpm_mutex); > > I think I would invert this. UNLOCKED is the exceptional case, so I'd > make the 0 flags lock. If we see UNLOCKED in the caller then we know > to audit for locking, 0 is much less obvious. I'm fine with either way. > > @@ -576,7 +576,7 @@ static int tpm2_load(struct tpm_chip *chip, > > goto out; > > } > > > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob"); > > + rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob", 0); > > All these points should accept a flags too and the caller should pass > in the TPM_TRASNMIT_UNLOCKED if it needs it.. For this bug fix it makes sense to implement it the way I did because it needs to be applied to multiple releases (I think I've underlined this in my changelog). If you think this is high priority, I can make the next revision into patch set of two patches. The second patch would implement the change you suggested. /Jarkko
WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> To: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> Cc: open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "moderated list:TPM DEVICE DRIVER" <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> Subject: Re: [PATCH] tpm: fix a race condition tpm2_unseal_trusted() Date: Thu, 25 Aug 2016 17:06:10 -0400 [thread overview] Message-ID: <20160825210437.GA8658@intel.com> (raw) In-Reply-To: <20160825183059.GB1142-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> On Thu, Aug 25, 2016 at 12:30:59PM -0600, Jason Gunthorpe wrote: > On Tue, Aug 23, 2016 at 08:57:22PM -0400, Jarkko Sakkinen wrote: > > > + if (flags & TPM_TRANSMIT_LOCK) > > + mutex_lock(&chip->tpm_mutex); > > I think I would invert this. UNLOCKED is the exceptional case, so I'd > make the 0 flags lock. If we see UNLOCKED in the caller then we know > to audit for locking, 0 is much less obvious. I'm fine with either way. > > @@ -576,7 +576,7 @@ static int tpm2_load(struct tpm_chip *chip, > > goto out; > > } > > > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob"); > > + rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob", 0); > > All these points should accept a flags too and the caller should pass > in the TPM_TRASNMIT_UNLOCKED if it needs it.. For this bug fix it makes sense to implement it the way I did because it needs to be applied to multiple releases (I think I've underlined this in my changelog). If you think this is high priority, I can make the next revision into patch set of two patches. The second patch would implement the change you suggested. /Jarkko ------------------------------------------------------------------------------
next prev parent reply other threads:[~2016-08-25 21:18 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-24 0:57 [PATCH] tpm: fix a race condition tpm2_unseal_trusted() Jarkko Sakkinen 2016-08-24 0:57 ` Jarkko Sakkinen 2016-08-24 1:32 ` Jarkko Sakkinen 2016-08-24 1:32 ` Jarkko Sakkinen 2016-08-24 1:32 ` Jarkko Sakkinen 2016-08-25 18:30 ` Jason Gunthorpe 2016-08-25 18:30 ` Jason Gunthorpe 2016-08-25 18:30 ` Jason Gunthorpe 2016-08-25 21:06 ` Jarkko Sakkinen [this message] 2016-08-25 21:06 ` Jarkko Sakkinen 2016-08-25 21:06 ` Jarkko Sakkinen 2016-08-25 21:09 ` Jason Gunthorpe 2016-08-25 21:09 ` Jason Gunthorpe 2016-08-25 21:09 ` Jason Gunthorpe -- strict thread matches above, loose matches on Subject: below -- 2016-08-16 19:38 Jarkko Sakkinen 2016-08-16 19:38 ` Jarkko Sakkinen 2016-08-17 4:31 ` Jarkko Sakkinen 2016-08-17 4:31 ` Jarkko Sakkinen 2016-07-20 0:16 Jarkko Sakkinen 2016-07-20 0:16 ` Jarkko Sakkinen 2016-07-20 16:48 ` Jason Gunthorpe 2016-07-20 16:48 ` Jason Gunthorpe 2016-07-20 20:53 ` Jarkko Sakkinen 2016-07-20 20:53 ` Jarkko Sakkinen 2016-07-20 21:13 ` Jason Gunthorpe 2016-07-20 21:13 ` Jason Gunthorpe 2016-07-21 9:02 ` Jarkko Sakkinen 2016-07-21 9:02 ` Jarkko Sakkinen 2016-07-21 16:25 ` Jason Gunthorpe 2016-07-21 16:25 ` Jason Gunthorpe 2016-08-09 10:36 ` Jarkko Sakkinen 2016-08-09 10:36 ` Jarkko Sakkinen 2016-08-09 15:49 ` Jason Gunthorpe 2016-08-09 15:49 ` Jason Gunthorpe
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=20160825210437.GA8658@intel.com \ --to=jarkko.sakkinen@linux.intel.com \ --cc=jgunthorpe@obsidianresearch.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=peterhuewe@gmx.de \ --cc=stable@vger.kernel.org \ --cc=tpmdd-devel@lists.sourceforge.net \ --cc=tpmdd@selhorst.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.