From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.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 12:30:59 -0600 [thread overview] Message-ID: <20160825183059.GB1142@obsidianresearch.com> (raw) In-Reply-To: <1472000243-7088-1-git-send-email-jarkko.sakkinen@linux.intel.com> 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. > @@ -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.. > + mutex_lock(&chip->tpm_mutex); > rc = tpm2_load(chip, payload, options, &blob_handle); > if (rc) So when we read here we see the pattern: > + mutex_lock(&chip->tpm_mutex); > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRASNMIT_UNLOCKED); Which is much easier to audit.. Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> To: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@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 12:30:59 -0600 [thread overview] Message-ID: <20160825183059.GB1142@obsidianresearch.com> (raw) In-Reply-To: <1472000243-7088-1-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 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. > @@ -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.. > + mutex_lock(&chip->tpm_mutex); > rc = tpm2_load(chip, payload, options, &blob_handle); > if (rc) So when we read here we see the pattern: > + mutex_lock(&chip->tpm_mutex); > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRASNMIT_UNLOCKED); Which is much easier to audit.. Jason ------------------------------------------------------------------------------
next prev parent reply other threads:[~2016-08-25 18:33 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 [this message] 2016-08-25 18:30 ` Jason Gunthorpe 2016-08-25 18:30 ` Jason Gunthorpe 2016-08-25 21:06 ` Jarkko Sakkinen 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=20160825183059.GB1142@obsidianresearch.com \ --to=jgunthorpe@obsidianresearch.com \ --cc=jarkko.sakkinen@linux.intel.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.