Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()

[Jerry Snitselaar]

On Mon Sep 16 19, Jerry Snitselaar wrote:
> On Mon Sep 16 19, Jarkko Sakkinen wrote:
> > tpm_send() does not give anymore the result back to the caller. This
> > would require another memcpy(), which kind of tells that the whole
> > approach is somewhat broken. Instead, as Mimi suggested, this commit
> > just wraps the data to the tpm_buf, and thus the result will not go to
> > the garbage.
> >
> > Obviously this assumes from the caller that it passes large enough
> > buffer, which makes the whole API somewhat broken because it could be
> > different size than @buflen but since trusted keys is the only module
> > using this API right now I think that this fix is sufficient for the
> > moment.
> >
> > In the near future the plan is to replace the parameters with a tpm_buf
> > created by the caller.
> >
> > Reported-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> > Suggested-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> > ---
> > drivers/char/tpm/tpm-interface.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index d9ace5480665..2459d36dd8cc 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> > 	if (!chip)
> > 		return -ENODEV;
> >
> > -	rc = tpm_buf_init(&buf, 0, 0);
> > -	if (rc)
> > -		goto out;
> > -
> > -	memcpy(buf.data, cmd, buflen);
> > +	buf.data = cmd;
> > 	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to a send a command");
> > -	tpm_buf_destroy(&buf);
> > +
> > out:
> > 	tpm_put_ops(chip);
> > 	return rc;
> > --
> > 2.20.1
>
> Nothing uses the out label any longer so it should be dropped as well, but other than that...
>
> Acked-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>

sigh (wrong emacs macro hit), that should be:

Reviewed-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>