Commit 3d7a850f authored by Jarkko Sakkinen's avatar Jarkko Sakkinen

tpm/tpm_crb: Avoid unaligned reads in crb_recv()

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 170d13ca, 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: 30fc8d13 ("tpm: TPM 2.0 CRB Interface")
Signed-off-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: default avatarJerry Snitselaar <jsnitsel@redhat.com>
Acked-by: default avatarTomas Winkler <tomas.winkler@intel.com>
parent 2181e084
...@@ -287,19 +287,29 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count) ...@@ -287,19 +287,29 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
struct crb_priv *priv = dev_get_drvdata(&chip->dev); struct crb_priv *priv = dev_get_drvdata(&chip->dev);
unsigned int expected; unsigned int expected;
/* sanity check */ /* A sanity check that the upper layer wants to get at least the header
if (count < 6) * as that is the minimum size for any TPM response.
*/
if (count < TPM_HEADER_SIZE)
return -EIO; return -EIO;
/* If this bit is set, according to the spec, the TPM is in
* unrecoverable condition.
*/
if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR) if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
return -EIO; return -EIO;
memcpy_fromio(buf, priv->rsp, 6); /* Read the first 8 bytes in order to get the length of the response.
expected = be32_to_cpup((__be32 *) &buf[2]); * We read exactly a quad word in order to make sure that the remaining
if (expected > count || expected < 6) * reads will be aligned.
*/
memcpy_fromio(buf, priv->rsp, 8);
expected = be32_to_cpup((__be32 *)&buf[2]);
if (expected > count || expected < TPM_HEADER_SIZE)
return -EIO; return -EIO;
memcpy_fromio(&buf[6], &priv->rsp[6], expected - 6); memcpy_fromio(&buf[8], &priv->rsp[8], expected - 8);
return expected; return expected;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment