tpmdd-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: <Alexander.Steffen@infineon.com>
To: jarkko.sakkinen@linux.intel.com
Cc: tpmdd-devel@lists.sourceforge.net, stable@vger.kernel.org
Subject: RE: [PATCH RESEND 1/2] tpm-dev-common: Reject too short writes
Date: Mon, 4 Sep 2017 17:35:00 +0000	[thread overview]
Message-ID: <dd865ba5751e4d568709e0aaf209633b@MUCSE603.infineon.com> (raw)
In-Reply-To: <20170830104059.sglgybn3fxh24qjr@linux.intel.com>

> On Mon, Aug 28, 2017 at 05:11:00PM +0000,
> Alexander.Steffen@infineon.com wrote:
> > > > diff --git a/drivers/char/tpm/tpm-dev-common.c
> > > > b/drivers/char/tpm/tpm-dev-common.c
> > > > index 610638a..c39b581 100644
> > > > --- a/drivers/char/tpm/tpm-dev-common.c
> > > > +++ b/drivers/char/tpm/tpm-dev-common.c
> > > > @@ -119,7 +119,7 @@ ssize_t tpm_common_write(struct file *file,
> > > > const
> > > char __user *buf,
> > > >  		return -EPIPE;
> > > >  	}
> > > >  	out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
> > > > -				sizeof(priv->data_buffer), 0);
> > > > +				sizeof(priv->data_buffer), in_size, 0);
> > >
> > > Why you couldn't just
> > >
> > > unsigned int bufsiz;
> > >
> > > /* ... */
> > >
> > > bufsiz = sizeof(priv->data_buffer);
> > > if (in_size < bufsiz)
> > > 	bufsiz = in_size;
> > >
> > > out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
> > > bufsiz, 0);
> >
> > Because the code needs to know both how large the buffer is (in order to
> avoid buffer overflows when writing to it) and how much of the data in the
> buffer is valid (in order not to send random junk to the TPM). This is made
> more explicit in PATCH 2/2.
> >
> > Your example fails as soon as the response is longer than the command.
> >
> > Alexander
> 
> Got you.
> 
> Do the comparison for count tpm-dev-common.c as it is the only call site
> where this is needed instead of scrabbling with the parameters. In other call
> sites this is unnecessary at this point.
> 
> This will also make backporting a factor more sleek.

I am not entirely happy with that approach, since it leads to worse code, splitting the buffer size validation logic over multiple places, but I can see how it makes backporting easier thanks to a smaller diff. A new patch is on its way.

Maybe at some point those parts of the code and the internal interfaces should be thoroughly cleaned up (i.e. rewritten).

Alexander

  reply	other threads:[~2017-09-04 17:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-24  8:35 [PATCH RESEND 0/2] Avoid sending invalid data to the TPM Alexander Steffen
     [not found] ` <20170824083545.13280-1-Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org>
2017-08-24  8:35   ` [PATCH RESEND 1/2] tpm-dev-common: Reject too short writes Alexander Steffen
2017-08-25 16:54     ` Jarkko Sakkinen
     [not found]       ` <20170825165451.b7lv7t5w3nhbz7da-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-08-28 17:11         ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
2017-08-30 10:40           ` Jarkko Sakkinen
2017-09-04 17:35             ` Alexander.Steffen [this message]
     [not found]               ` <dd865ba5751e4d568709e0aaf209633b-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>
2017-09-05 19:02                 ` Jarkko Sakkinen
2017-08-24  8:35   ` [PATCH RESEND 2/2] tpm-interface: Fix checks of buffer size Alexander Steffen
2017-08-25 16:44   ` [PATCH RESEND 0/2] Avoid sending invalid data to the TPM Jarkko Sakkinen
     [not found]     ` <20170825164416.svo7khm4zsmosxbx-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-09-01 12:08       ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w

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=dd865ba5751e4d568709e0aaf209633b@MUCSE603.infineon.com \
    --to=alexander.steffen@infineon.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=tpmdd-devel@lists.sourceforge.net \
    --subject='RE: [PATCH RESEND 1/2] tpm-dev-common: Reject too short writes' \
    /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: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).