Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     linux-integrity@vger.kernel.org
Cc:     Jason Gunthorpe <jgg@nvidia.com>,
        Alejandro Cabrera <alejandro.cabreraaldaya@tuni.fi>,
        Jarkko Sakkinen <jarkko.sakkinen@tuni.fi>,
        stable@vger.kernel.org, Jarkko Sakkinen <jarkko@kernel.org>,
        Stefan Berger <stefanb@linux.vnet.ibm.com>,
        linux-kernel@vger.kernel.org
Subject: [PATCH] tpm: factor out the user space mm from tpm_vtpm_set_locality()
Date:   Tue, 30 May 2023 23:50:01 +0300
Message-Id: <20230530205001.1302975-1-jarkko@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Jarkko Sakkinen <jarkko.sakkinen@tuni.fi>

vtpm_proxy_fops_set_locality() causes kernel buffers to be passed to
copy_from_user() and copy_to_user().

Factor out the crippled code away with help of an internal API for
managing struct proxy_dev instances.

Link: https://lore.kernel.org/all/20230530020133.235765-1-jarkko@kernel.org/
Cc: stable@vger.kernel.org # v4.14+
Fixes: be4c9acfe297 ("tpm: vtpm_proxy: Implement request_locality function.")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@tuni.fi>
---
I've tested this on RISC-V QEMU and have not noticed issues so far, and
thus dropped the RFC tag. I've been looking into the code a lot lately
as I'm building a boot time support for it, which will allow to e.g.
test IMA without a physical TPM.	
 drivers/char/tpm/tpm_vtpm_proxy.c | 168 ++++++++++++++++--------------
 1 file changed, 87 insertions(+), 81 deletions(-)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 30e953988cab..83496742cc19 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -38,7 +38,6 @@ struct proxy_dev {
 #define STATE_OPENED_FLAG        BIT(0)
 #define STATE_WAIT_RESPONSE_FLAG BIT(1)  /* waiting for emulator response */
 #define STATE_REGISTERED_FLAG	 BIT(2)
-#define STATE_DRIVER_COMMAND     BIT(3)  /* sending a driver specific command */
 
 	size_t req_len;              /* length of queued TPM request */
 	size_t resp_len;             /* length of queued TPM response */
@@ -58,6 +57,53 @@ static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev);
  * Functions related to 'server side'
  */
 
+static ssize_t __vtpm_proxy_copy_from(struct proxy_dev *proxy_dev, u8 *buf, size_t count)
+{
+	size_t len = proxy_dev->req_len;
+
+	if (!(proxy_dev->state & STATE_OPENED_FLAG))
+		return -EPIPE;
+
+	if (count < len || len > sizeof(proxy_dev->buffer)) {
+		pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n", count, len);
+		return -EIO;
+	}
+
+	memcpy(buf, proxy_dev->buffer, len);
+	memset(proxy_dev->buffer, 0, len);
+	proxy_dev->req_len = 0;
+	proxy_dev->state |= STATE_WAIT_RESPONSE_FLAG;
+
+	return len;
+}
+
+static ssize_t __vtpm_proxy_copy_to(struct proxy_dev *proxy_dev, const u8 *buf, size_t count)
+{
+	if (!(proxy_dev->state & STATE_OPENED_FLAG))
+		return -EPIPE;
+
+	if (count > sizeof(proxy_dev->buffer) || !(proxy_dev->state & STATE_WAIT_RESPONSE_FLAG))
+		return -EIO;
+
+	proxy_dev->state &= ~STATE_WAIT_RESPONSE_FLAG;
+	proxy_dev->req_len = 0;
+	proxy_dev->resp_len = count;
+	memcpy(proxy_dev->buffer, buf, count);
+
+	return count;
+}
+
+static int vtpm_proxy_wait_for(struct proxy_dev *proxy_dev)
+{
+	if (wait_event_interruptible(
+		proxy_dev->wq,
+		proxy_dev->req_len != 0 || !(proxy_dev->state & STATE_OPENED_FLAG)))
+		return -EINTR;
+
+	return 0;
+}
+
+
 /**
  * vtpm_proxy_fops_read - Read TPM commands on 'server side'
  *
@@ -73,44 +119,26 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 				    size_t count, loff_t *off)
 {
 	struct proxy_dev *proxy_dev = filp->private_data;
-	size_t len;
-	int sig, rc;
+	u8 *kernel_buf;
+	ssize_t rc;
 
-	sig = wait_event_interruptible(proxy_dev->wq,
-		proxy_dev->req_len != 0 ||
-		!(proxy_dev->state & STATE_OPENED_FLAG));
-	if (sig)
-		return -EINTR;
-
-	mutex_lock(&proxy_dev->buf_lock);
-
-	if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
-		mutex_unlock(&proxy_dev->buf_lock);
-		return -EPIPE;
-	}
-
-	len = proxy_dev->req_len;
-
-	if (count < len || len > sizeof(proxy_dev->buffer)) {
-		mutex_unlock(&proxy_dev->buf_lock);
-		pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n",
-			 count, len);
-		return -EIO;
-	}
-
-	rc = copy_to_user(buf, proxy_dev->buffer, len);
-	memset(proxy_dev->buffer, 0, len);
-	proxy_dev->req_len = 0;
+	rc = vtpm_proxy_wait_for(proxy_dev);
+	if (rc)
+		return rc;
 
-	if (!rc)
-		proxy_dev->state |= STATE_WAIT_RESPONSE_FLAG;
+	kernel_buf = kzalloc(sizeof(proxy_dev->buffer), GFP_KERNEL);
+	if (!kernel_buf)
+		return -ENOMEM;
 
+	mutex_lock(&proxy_dev->buf_lock);
+	rc = __vtpm_proxy_copy_from(proxy_dev, buf, count);
 	mutex_unlock(&proxy_dev->buf_lock);
 
-	if (rc)
-		return -EFAULT;
+	if (!rc && copy_to_user(buf, kernel_buf, count))
+		rc = -EFAULT;
 
-	return len;
+	kfree(kernel_buf);
+	return rc;
 }
 
 /**
@@ -128,36 +156,26 @@ static ssize_t vtpm_proxy_fops_write(struct file *filp, const char __user *buf,
 				     size_t count, loff_t *off)
 {
 	struct proxy_dev *proxy_dev = filp->private_data;
+	u8 *kernel_buf;
+	int rc;
 
-	mutex_lock(&proxy_dev->buf_lock);
-
-	if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
-		mutex_unlock(&proxy_dev->buf_lock);
-		return -EPIPE;
-	}
-
-	if (count > sizeof(proxy_dev->buffer) ||
-	    !(proxy_dev->state & STATE_WAIT_RESPONSE_FLAG)) {
-		mutex_unlock(&proxy_dev->buf_lock);
-		return -EIO;
-	}
-
-	proxy_dev->state &= ~STATE_WAIT_RESPONSE_FLAG;
-
-	proxy_dev->req_len = 0;
+	kernel_buf = kzalloc(sizeof(proxy_dev->buffer), GFP_KERNEL);
+	if (!kernel_buf)
+		return -ENOMEM;
 
-	if (copy_from_user(proxy_dev->buffer, buf, count)) {
-		mutex_unlock(&proxy_dev->buf_lock);
+	if (copy_from_user(kernel_buf, buf, count)) {
+		kfree(kernel_buf);
 		return -EFAULT;
 	}
 
-	proxy_dev->resp_len = count;
-
+	mutex_lock(&proxy_dev->buf_lock);
+	rc = __vtpm_proxy_copy_to(proxy_dev, kernel_buf, count);
 	mutex_unlock(&proxy_dev->buf_lock);
 
 	wake_up_interruptible(&proxy_dev->wq);
+	kfree(kernel_buf);
 
-	return count;
+	return rc;
 }
 
 /*
@@ -295,28 +313,6 @@ static int vtpm_proxy_tpm_op_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	return len;
 }
 
-static int vtpm_proxy_is_driver_command(struct tpm_chip *chip,
-					u8 *buf, size_t count)
-{
-	struct tpm_header *hdr = (struct tpm_header *)buf;
-
-	if (count < sizeof(struct tpm_header))
-		return 0;
-
-	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		switch (be32_to_cpu(hdr->ordinal)) {
-		case TPM2_CC_SET_LOCALITY:
-			return 1;
-		}
-	} else {
-		switch (be32_to_cpu(hdr->ordinal)) {
-		case TPM_ORD_SET_LOCALITY:
-			return 1;
-		}
-	}
-	return 0;
-}
-
 /*
  * Called when core TPM driver forwards TPM requests to 'server side'.
  *
@@ -330,6 +326,7 @@ static int vtpm_proxy_is_driver_command(struct tpm_chip *chip,
 static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+	unsigned int ord = ((struct tpm_header *)buf)->ordinal;
 
 	if (count > sizeof(proxy_dev->buffer)) {
 		dev_err(&chip->dev,
@@ -338,8 +335,10 @@ static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
 		return -EIO;
 	}
 
-	if (!(proxy_dev->state & STATE_DRIVER_COMMAND) &&
-	    vtpm_proxy_is_driver_command(chip, buf, count))
+	if ((chip->flags & TPM_CHIP_FLAG_TPM2) && ord == TPM2_CC_SET_LOCALITY)
+		return -EFAULT;
+
+	if (ord == TPM_ORD_SET_LOCALITY)
 		return -EFAULT;
 
 	mutex_lock(&proxy_dev->buf_lock);
@@ -407,13 +406,20 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
 				  TPM_ORD_SET_LOCALITY);
 	if (rc)
 		return rc;
+
 	tpm_buf_append_u8(&buf, locality);
 
-	proxy_dev->state |= STATE_DRIVER_COMMAND;
+	mutex_lock(&proxy_dev->buf_lock);
+	rc = __vtpm_proxy_copy_to(proxy_dev, buf.data, tpm_buf_length(&buf));
+	mutex_unlock(&proxy_dev->buf_lock);
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to set locality");
+	rc = vtpm_proxy_wait_for(proxy_dev);
+	if (rc)
+		return rc;
 
-	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
+	mutex_lock(&proxy_dev->buf_lock);
+	rc = __vtpm_proxy_copy_from(proxy_dev, buf.data, tpm_buf_length(&buf));
+	mutex_unlock(&proxy_dev->buf_lock);
 
 	if (rc < 0) {
 		locality = rc;
-- 
2.39.2