From: "Winkler, Tomas" <tomas.winkler@intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: "tpmdd-devel@lists.sourceforge.net"
<tpmdd-devel@lists.sourceforge.net>,
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] tpm: use get_unaligned_be32 unaligned buffer access.
Date: Wed, 23 Nov 2016 20:56:25 +0000 [thread overview]
Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B543308CB@hasmsx108.ger.corp.intel.com> (raw)
In-Reply-To: <20161123165734.GA2763@obsidianresearch.com>
> 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.
Yes, compiler takes care at performance penalty but probably we don't care about that much,
and readability is maybe more important here. What I've done is pretty much mechanical fix,
I agree this is better approach in this particular case.
> > @@ -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
next prev parent reply other threads:[~2016-11-23 20:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-23 11:04 [PATCH] tpm: use get_unaligned_be32 unaligned buffer access Tomas Winkler
2016-11-23 16:57 ` Jason Gunthorpe
2016-11-23 20:56 ` Winkler, Tomas [this message]
2016-11-23 21:01 ` Jason Gunthorpe
2016-11-24 13:34 ` Jarkko Sakkinen
2016-12-03 15:30 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5B8DA87D05A7694D9FA63FD143655C1B543308CB@hasmsx108.ger.corp.intel.com \
--to=tomas.winkler@intel.com \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=jgunthorpe@obsidianresearch.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tpmdd-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).