()
On Tue, Feb 05, 2019 at 10:57:19PM +0200, Jarkko Sakkinen wrote:
On Tue, Feb 05, 2019 at 02:56:02PM +0000, Winkler, Tomas wrote:
-----Original Message----- From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com] Sent: Tuesday, February 05, 2019 16:36 To: Winkler, Tomas tomas.winkler@intel.com Cc: linux-integrity@vger.kernel.org; linux-kernel@vger.kernel.org; linux- security-module@vger.kernel.org; stable@vger.kernel.org; James Morris jmorris@namei.org; Jerry Snitselaar jsnitsel@redhat.com 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:
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.
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 tomas.winkler@intel.com Cc: Jerry Snitselaar jsnitsel@redhat.com Fixes: 30fc8d138e91 ("tpm: TPM 2.0 CRB Interface") Signed-off-by: Jarkko Sakkinen jarkko.sakkinen@linux.intel.com Reviewed-by: Jerry Snitselaar jsnitsel@redhat.com
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
- unrecovable -> unrecoverable
- /* Read 8 bytes (not just 6 bytes, which would cover the tag and
the response length
* fields) in order to make sure that the remaining memory+accesses */
Thanks and apologies for missing these.
Fixed comments and applied the patch, thank you. Do I amend your acked-by?
Please, do. Thanks Tomas