Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1649963 > unrolled thread

[PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace from sending driver command

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace from sending driver command Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-05-24 23:50 +0200
    Re: [PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace from sending  driver command Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-05-25 00:30 +0200
      Re: [tpmdd-devel] [PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace  from sending driver command Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-05-25 02:20 +0200

#1649963 — [PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace from sending driver command

To prevent userspace from sending the TPM driver command to set
the locality, we need to check every command that is sent from
user space. To distinguish user space commands from internally
sent commands we introduce an additional state flag
STATE_DRIVER_COMMAND that is set while the driver sends this
command. Similar to the TPM 2 space commands we return an error
code when this command is detected.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 66024bf..1d877cc 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -43,6 +43,7 @@ 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 */
@@ -299,6 +300,28 @@ 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_input_header *hdr = (struct tpm_input_header *)buf;
+
+	if (count < sizeof(struct tpm_input_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'.
  *
@@ -321,6 +344,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))
+		return -EFAULT;
+
 	mutex_lock(&proxy_dev->buf_lock);
 
 	if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
@@ -376,6 +403,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
 	struct tpm_buf buf;
 	int rc;
 	const struct tpm_output_header *header;
+	struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
@@ -387,9 +415,14 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
 		return rc;
 	tpm_buf_append_u8(&buf, locality);
 
+	proxy_dev->state |= STATE_DRIVER_COMMAND;
+
 	rc = tpm_transmit_cmd(chip, NULL, buf.data, tpm_buf_length(&buf), 0,
 			      TPM_TRANSMIT_UNLOCKED | TPM_TRANSMIT_RAW,
 			      "attempting to set locality");
+
+	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
+
 	if (rc < 0) {
 		locality = rc;
 		goto out;
-- 
2.4.3

[toc] | [next] | [standalone]

#1650003 — Re: [PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace from sending driver command

On Wed, May 24, 2017 at 05:39:41PM -0400, Stefan Berger wrote:
> To prevent userspace from sending the TPM driver command to set
> the locality, we need to check every command that is sent from
> user space. To distinguish user space commands from internally
> sent commands we introduce an additional state flag
> STATE_DRIVER_COMMAND that is set while the driver sends this
> command. Similar to the TPM 2 space commands we return an error
> code when this command is detected.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm_vtpm_proxy.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 66024bf..1d877cc 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -43,6 +43,7 @@ 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 */
> @@ -299,6 +300,28 @@ 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_input_header *hdr = (struct tpm_input_header *)buf;
> +
> +	if (count < sizeof(struct tpm_input_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'.
>   *
> @@ -321,6 +344,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))
> +		return -EFAULT;
> +
>  	mutex_lock(&proxy_dev->buf_lock);
>  
>  	if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> @@ -376,6 +403,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>  	struct tpm_buf buf;
>  	int rc;
>  	const struct tpm_output_header *header;
> +	struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> @@ -387,9 +415,14 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>  		return rc;
>  	tpm_buf_append_u8(&buf, locality);
>  
> +	proxy_dev->state |= STATE_DRIVER_COMMAND;
> +
>  	rc = tpm_transmit_cmd(chip, NULL, buf.data, tpm_buf_length(&buf), 0,
>  			      TPM_TRANSMIT_UNLOCKED | TPM_TRANSMIT_RAW,
>  			      "attempting to set locality");
> +
> +	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
> +
>  	if (rc < 0) {
>  		locality = rc;
>  		goto out;
> -- 
> 2.4.3
> 

Otherwise fine except for the redundant code.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1650055 — Re: [tpmdd-devel] [PATCH v6 3/3] tpm: vtpm_proxy: Prevent userspace from sending driver command

On Wed, May 24, 2017 at 03:22:11PM -0700, Jarkko Sakkinen wrote:
> On Wed, May 24, 2017 at 05:39:41PM -0400, Stefan Berger wrote:
> > To prevent userspace from sending the TPM driver command to set
> > the locality, we need to check every command that is sent from
> > user space. To distinguish user space commands from internally
> > sent commands we introduce an additional state flag
> > STATE_DRIVER_COMMAND that is set while the driver sends this
> > command. Similar to the TPM 2 space commands we return an error
> > code when this command is detected.
> > 
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > ---
> >  drivers/char/tpm/tpm_vtpm_proxy.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > index 66024bf..1d877cc 100644
> > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > @@ -43,6 +43,7 @@ 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 */
> > @@ -299,6 +300,28 @@ 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_input_header *hdr = (struct tpm_input_header *)buf;
> > +
> > +	if (count < sizeof(struct tpm_input_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'.
> >   *
> > @@ -321,6 +344,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))
> > +		return -EFAULT;
> > +
> >  	mutex_lock(&proxy_dev->buf_lock);
> >  
> >  	if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> > @@ -376,6 +403,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> >  	struct tpm_buf buf;
> >  	int rc;
> >  	const struct tpm_output_header *header;
> > +	struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
> >  
> >  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >  		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> > @@ -387,9 +415,14 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> >  		return rc;
> >  	tpm_buf_append_u8(&buf, locality);
> >  
> > +	proxy_dev->state |= STATE_DRIVER_COMMAND;
> > +
> >  	rc = tpm_transmit_cmd(chip, NULL, buf.data, tpm_buf_length(&buf), 0,
> >  			      TPM_TRANSMIT_UNLOCKED | TPM_TRANSMIT_RAW,
> >  			      "attempting to set locality");
> > +
> > +	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
> > +
> >  	if (rc < 0) {
> >  		locality = rc;
> >  		goto out;
> > -- 
> > 2.4.3
> > 
> 
> Otherwise fine except for the redundant code.
> 
> /Jarkko

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Probably not yet merging and testing this this week though as I'm
travelling in US right now.

/Jarkko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web