Re: [PATCH v3] tpm/tpm_crb: Avoid unaligned reads in crb_recv()

On Tue, Feb 05, 2019 at 02:56:02PM +0000, Winkler, Tomas wrote:

quoted

-----Original Message-----
From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
Sent: Tuesday, February 05, 2019 16:36
To: Winkler, Tomas <redacted>
Cc: linux-integrity@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
security-module@vger.kernel.org; stable@vger.kernel.org; James Morris
[off-list ref]; Jerry Snitselaar [off-list ref]
Subject: Re: [PATCH v3] tpm/tpm_crb: Avoid unaligned reads in crb_recv()

On Tue, Feb 05, 2019 at 11:07:16AM +0000, Winkler, Tomas wrote:

quoted

quoted

The current approach to read first 6 bytes from the response and
then tail of the response, can cause the 2nd memcpy_fromio() to do
an unaligned read (e.g. read 32-bit word from address aligned to a
16-bits), depending on how
memcpy_fromio() is implemented. If this happens, the read will fail
and the memory controller will fill the read with 1's.

This was triggered by 170d13ca3a2f, which should be probably refined
to check and react to the address alignment. Before that commit, on
x86
memcpy_fromio() turned out to be memcpy(). By a luck GCC has done
the right thing (from tpm_crb's perspective) for us so far, but we should not

rely on that.

quoted

quoted

Thus, it makes sense to fix this also in tpm_crb, not least because
the fix can be then backported to stable kernels and make them more
robust when compiled in differing environments.

Cc: stable@vger.kernel.org
Cc: James Morris <jmorris@namei.org>
Cc: Tomas Winkler <redacted>
Cc: Jerry Snitselaar <redacted>
Fixes: 30fc8d138e91 ("tpm: TPM 2.0 CRB Interface")
Signed-off-by: Jarkko Sakkinen <redacted>
Reviewed-by: Jerry Snitselaar <redacted>
---
v3:
* Fix typo i.e. %s/reminding/remaining/g

Why you haven't fixed all the typos I've pointed out? I think you missed that.

I saw only comment about remaining. Was there something else? Can fix.

https://www.spinics.net/lists/stable/msg283648.html

1. unrecovable -> unrecoverable
2. /* Read 8 bytes (not just 6 bytes, which would cover the tag and the response  length

quoted

+	 * fields) in order to make sure that the remaining memory accesses */ 

Thanks and apologies for missing these.

/Jarkko