Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933774AbcKWQ5o (ORCPT ); Wed, 23 Nov 2016 11:57:44 -0500 Received: from quartz.orcorp.ca ([184.70.90.242]:56605 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932358AbcKWQ5n (ORCPT ); Wed, 23 Nov 2016 11:57:43 -0500 Date: Wed, 23 Nov 2016 09:57:35 -0700 From: Jason Gunthorpe To: Tomas Winkler Cc: tpmdd-devel@lists.sourceforge.net, Jarkko Sakkinen , linux-kernel@vger.kernel.org Subject: Re: [PATCH] tpm: use get_unaligned_be32 unaligned buffer access. Message-ID: <20161123165734.GA2763@obsidianresearch.com> References: <1479899094-9486-1-git-send-email-tomas.winkler@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1479899094-9486-1-git-send-email-tomas.winkler@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.151 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2981 Lines: 97 On Wed, Nov 23, 2016 at 01:04:54PM +0200, Tomas Winkler wrote: > Use get_unaligned_be32 as b32_to_cpu doesn't work correctly on > all platforms for unaligned access. > > The fix doesn't cover all the cases as also some cast > structures have members on unaligned addresses. I think this is a good idea.. > @@ -353,8 +353,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz, > if (bufsiz > TPM_BUFSIZE) > bufsiz = TPM_BUFSIZE; > > - count = be32_to_cpu(*((__be32 *) (buf + 2))); > - ordinal = be32_to_cpu(*((__be32 *) (buf + 6))); > + count = get_unaligned_be32(buf + 2); > + ordinal = get_unaligned_be32(buf + 6); But lets fix this better and get rid of the constants too... const tpm_input_header *hdr = buf; count = be32_to_cpu(hdr->length); ordinal = be32_to_cpu(hdr->ordinal); Compiler will take care of unaligned for __packed. > @@ -178,7 +178,7 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count) > return -EIO; > > memcpy_fromio(buf, priv->rsp, 6); > - expected = be32_to_cpup((__be32 *) &buf[2]); > + expected = get_unaligned_be32(buf + 2); Here too, except tpm_output_header (and is tpm1 and 2 the same here?) > @@ -451,7 +452,7 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count) > goto out; > } > > - expected = be32_to_cpu(*(__be32 *)(buf + 2)); > + expected = get_unaligned_be32(buf + 2); Ditto > @@ -314,7 +315,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count) > * convert number of expected bytes field from big endian 32 bit > * to machine native > */ > - expected = be32_to_cpu(*(__be32 *) (buf + 2)); > + expected = get_unaligned_be32(buf + 2); Ditto > if (expected > count) { > dev_err(dev, "%s() expected > count\n", __func__); > size = -EIO; > @@ -442,7 +443,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len) > i2c_nuvoton_ready(chip); > return rc; > } > - ordinal = be32_to_cpu(*((__be32 *) (buf + 6))); > + ordinal = get_unaligned_be32(buf + 6); Ditto > @@ -174,8 +173,7 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count) > return -EIO; > } > > - native_size = (__force __be32 *) (buf + 2); > - size = be32_to_cpu(*native_size); > + size = get_unaligned_be32(buf + 2); Ditto > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 7993678954a2..5323c54dc917 100644 > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -222,7 +222,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count) > goto out; > } > > - expected = be32_to_cpu(*(__be32 *) (buf + 2)); > + expected = get_unaligned_be32(buf + 2); Ditto > @@ -371,7 +371,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len) > goto out_err; > > if (chip->flags & TPM_CHIP_FLAG_IRQ) { > - ordinal = be32_to_cpu(*((__be32 *) (buf + 6))); > + ordinal = get_unaligned_be32(buf + 6); Ditto Jason