On Tue, Nov 13, 2018 at 02:08:39PM +0100, Roberto Sassu wrote:

On 11/8/2018 3:08 PM, Jarkko Sakkinen wrote:

quoted

On Tue, Nov 06, 2018 at 04:01:59PM +0100, Roberto Sassu wrote:

quoted

This patch protects against data corruption that could happen in the bus,
by checking that that the digest size returned by the TPM during a PCR read
matches the size of the algorithm passed as argument to tpm2_pcr_read().

This check is performed after information about the PCR banks has been
retrieved.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
  drivers/char/tpm/tpm2-cmd.c | 16 +++++++++++++++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e2d5b84286a7..3b0b5b032901 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c

@@ -187,15 +187,28 @@ struct tpm2_pcr_read_out {
  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx,
  		  struct tpm_digest *digest_struct, u16 *digest_size_ptr)
  {
+	int i;
  	int rc;
  	struct tpm_buf buf;
  	struct tpm2_pcr_read_out *out;
  	u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
  	u16 digest_size;
+	u16 expected_digest_size = 0;
  	if (pcr_idx >= TPM2_PLATFORM_PCR)
  		return -EINVAL;
+	if (!digest_size_ptr) {
+		for (i = 0; i < chip->nr_active_banks &&
+		     chip->active_banks[i].alg_id != digest_struct->alg_id; i++)
+			;

Not sure if the semicolon should be in its own line.
`

quoted

+
+		if (i == chip->nr_active_banks)
+			return -EINVAL;
+
+		expected_digest_size = chip->active_banks[i].digest_size;
+	}
+
  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
  	if (rc)
  		return rc;

@@ -215,7 +228,8 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx,
  	out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
  	digest_size = be16_to_cpu(out->digest_size);
-	if (digest_size > sizeof(digest_struct->digest)) {
+	if (digest_size > sizeof(digest_struct->digest) ||
+	    (!digest_size_ptr && digest_size != expected_digest_size)) {
  		rc = -EINVAL;
  		goto out;
  	}

-- 
2.17.1

Please add

Cc: stable@vger.kernel.org.

Should I do the same for the previous patches? This patch cannot be
applied alone.

Roberto

No need. It is an issue that we deal with depenendent commits once it is
being backported. This could be dependent for example of a commit that
is even not in the series so does not make sense to do it now.

/Jarkko