From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: tpmdd-devel@lists.sourceforge.net,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7] tpm: Check size of response before accessing data
Date: Fri, 20 Jan 2017 18:21:58 +0200 [thread overview]
Message-ID: <20170120162145.lrdcrf5iw6d6otd3@intel.com> (raw)
In-Reply-To: <20170120133630.ld5shdqhquxzd7sn@intel.com>
On Fri, Jan 20, 2017 at 03:36:30PM +0200, Jarkko Sakkinen wrote:
> On Thu, Jan 19, 2017 at 07:19:12AM -0500, Stefan Berger wrote:
> > Make sure that we have not received less bytes than what is indicated
> > in the header of the TPM response. Also, check the number of bytes in
> > the response before accessing its data.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Oops. I found some odd stuff after all so hold on for a moment.
I could do these updates myself probably...
> > ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
> > - int len, unsigned int flags, const char *desc)
> > + size_t len, size_t min_rsp_body_length,
> > + unsigned int flags, const char *desc)
BTW, maybe the cmd_length would be actually a better idea because
it gets mixes witht local variable.
> > {
> > const struct tpm_output_header *header;
> > int err;
> > + ssize_t length;
Maybe it would make sense to name this as rsp_length.
> >
> > - len = tpm_transmit(chip, (const u8 *)cmd, len, flags);
> > - if (len < 0)
> > - return len;
> > - else if (len < TPM_HEADER_SIZE)
> > + length = tpm_transmit(chip, (const u8 *)cmd, len, flags);
> > + if (length < 0)
> > + return length;
> > + else if (length < TPM_HEADER_SIZE)
> > return -EFAULT;
> >
> > header = cmd;
> > + if (length < be32_to_cpu(header->length))
> > + return -EFAULT;
Why '<' and not '!='? In what legit case length would be larger?
> >
> > err = be32_to_cpu(header->return_code);
> > if (err != 0 && desc)
> > dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
> > desc);
> > + if (err)
> > + return err;
> >
> > - return err;
> > + if (be32_to_cpu(header->length) <
> > + min_rsp_body_length + TPM_HEADER_SIZE)
> > + return -EFAULT;
Why couldn't you use 'length' here?
/Jarkko
next prev parent reply other threads:[~2017-01-20 16:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-19 12:19 [PATCH v7] tpm: Check size of response before accessing data Stefan Berger
2017-01-19 12:19 ` Stefan Berger
2017-01-20 13:36 ` Jarkko Sakkinen
2017-01-20 13:36 ` Jarkko Sakkinen
2017-01-20 16:21 ` Jarkko Sakkinen [this message]
2017-01-20 17:55 ` Jarkko Sakkinen
2017-01-20 17:55 ` Jarkko Sakkinen
2017-01-20 20:14 ` Jarkko Sakkinen
2017-01-20 21:04 ` Stefan Berger
2017-01-20 21:04 ` Stefan Berger
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=20170120162145.lrdcrf5iw6d6otd3@intel.com \
--to=jarkko.sakkinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=stefanb@linux.vnet.ibm.com \
--cc=tpmdd-devel@lists.sourceforge.net \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.