linux-kernel - Re: [v8,09/10] tpm: Initialize TPM and get durations and timeoutsg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 29 Mar 2016 18:31:43 +0300
From:	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:	Stefan Berger <stefanb@...ux.vnet.ibm.com>
Cc:	tpmdd-devel@...ts.sourceforge.net, linux-doc@...r.kernel.org,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org
Subject: Re: [v8,09/10] tpm: Initialize TPM and get durations and timeoutsg

On Tue, Mar 22, 2016 at 06:54:30AM -0400, Stefan Berger wrote:
> On 03/22/2016 02:34 AM, Jarkko Sakkinen wrote:
> >On Sun, Mar 13, 2016 at 06:54:39PM -0400, Stefan Berger wrote:
> >>Add the retrieval of TPM 1.2 durations and timeouts. Since this requires
> >>the startup of the TPM, do this for TPM 1.2 and TPM 2.
> >>
> >>Signed-off-by: Stefan Berger <stefanb@...ux.vnet.ibm.com>
> >>CC: linux-kernel@...r.kernel.org
> >>CC: linux-doc@...r.kernel.org
> >>CC: linux-api@...r.kernel.org
> >>
> >>---
> >>drivers/char/tpm/tpm_vtpm_proxy.c | 95 +++++++++++++++++++++++++++++++++++----
> >>  1 file changed, 86 insertions(+), 9 deletions(-)
> >>
> >>diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> >>index 2bb2c8c..7fd686b 100644
> >>--- a/drivers/char/tpm/tpm_vtpm_proxy.c
> >>+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> >>@@ -45,8 +45,11 @@ struct proxy_dev {
> >>  	size_t req_len;              /* length of queued TPM request */
> >>  	size_t resp_len;             /* length of queued TPM response */
> >>  	u8 buffer[TPM_BUFSIZE];      /* request/response buffer */
> >>+
> >>+	struct work_struct work;     /* task that retrieves TPM timeouts */
> >>  };
> >>+static struct workqueue_struct *workqueue;
> >>  static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev);
> >>@@ -67,6 +70,15 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
> >>  	size_t len;
> >>  	int sig, rc;
> >>+	mutex_lock(&proxy_dev->buf_lock);
> >>+
> >>+	if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> >>+		mutex_unlock(&proxy_dev->buf_lock);
> >>+		return -EPIPE;
> >>+	}
> >>+
> >>+	mutex_unlock(&proxy_dev->buf_lock);
> >>+
> >>  	sig = wait_event_interruptible(proxy_dev->wq, proxy_dev->req_len != 0);
> >>  	if (sig)
> >>  		return -EINTR;
> >What if STATE_OPENED_FLAG is set after mutex_unlock()?
> 
> This flag is only set when the file descriptor for the server side is
> created (vtpm_proxy_fops_open()). After that it can only be cleared
> (vtpm_fops_undo_open()) due to an error condition, which then indicates to
> the server side that the file descriptor is now unusable. One error
> condition can for example be the failure by the TPM emulator to respond to
> the TPM_Startup with a success in the response.

You take the lock two times and OPENED flag could change in-between.

Why couldn't you put the call after wait_event_* after taking the lock?

/Jarkko