RE: [PATCH RESEND 1/2] tpm-dev-common: Reject too short writes

> On Mon, Aug 28, 2017 at 05:11:00PM +0000,
> [email protected] 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