tpmdd-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Andrey Pronin <apronin@chromium.org>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Peter Huewe <peterhuewe@gmx.de>,
	Marcel Selhorst <tpmdd@selhorst.net>,
	tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
	groeck@chromium.org, smbarber@chromium.org,
	dianders@chromium.org
Subject: Re: [PATCH 2/2] tpm: fix byte-order for the value read by tpm2_get_tpm_pt
Date: Fri, 15 Jul 2016 12:31:43 -0700	[thread overview]
Message-ID: <20160715193143.GA7052@apronin> (raw)
In-Reply-To: <20160715033553.GJ9347@obsidianresearch.com>

On Thu, Jul 14, 2016 at 09:35:53PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 08:17:01PM -0700, Andrey Pronin wrote:
> > conversion. The only place tpm2_get_tpm_pt() was used before was in
> > tpm2_gen_interrupt, which discarded the result. So, nobody noticed,
> > I guess.
> 
> If you have a moment can you run sparse on this file before/after this
> change and make sure it did complain and that there are not other
> endian errors?
> 

I ran sparse. In my case it didn't complain before or after the change.
It only complains about using cpu_to_be32() in tpm_capabilities and
tpm_sub_capabilities enum in tpm.h and the use of those enums in
tpm_getcap() in tpm-interface.c

I even tried changing the code there to
	__be32 vv = cmd.params.get_tpm_pt_out.value;
	u32 ret = vv;
to make it clear what's going on. Still, no complaints from sparse.

be32_to_cpu() should cleary be done in this case, though. As value
is defined as __be32 in struct tpm2_get_tpm_pt_out, and we return u32
in that parameter from tpm2_get_tpm_pt(). Plus, it's consistent with
the spec. It's just nobody has used this value yet in the existing
code, thus it went unnoticed.

To address "no dead code" comment, I'll drop defining various tpm2
properties and bits from this patchset and move it to the sysfs for
tpm2 patchset where they'll be needed for tpm2-specific attributes.
But this fix is valid on its own in my opinion.

  reply	other threads:[~2016-07-15 19:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-15  1:07 [PATCH 0/2] tpm: better support for 32-bit tpm2 properties Andrey Pronin
     [not found] ` <1468544838-9990-1-git-send-email-apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2016-07-15  1:07   ` [PATCH 1/2] tpm: define constants for " Andrey Pronin
     [not found]     ` <1468544838-9990-2-git-send-email-apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2016-07-15  3:08       ` Jason Gunthorpe
2016-07-18 18:58       ` Jarkko Sakkinen
2016-07-15  1:07   ` [PATCH 2/2] tpm: fix byte-order for the value read by tpm2_get_tpm_pt Andrey Pronin
2016-07-15  3:10     ` Jason Gunthorpe
     [not found]       ` <20160715031046.GC9347-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2016-07-15  3:17         ` Andrey Pronin
2016-07-15  3:35           ` Jason Gunthorpe
2016-07-15 19:31             ` Andrey Pronin [this message]
2016-07-15 19:39               ` Jason Gunthorpe
     [not found]                 ` <20160715193928.GA20719-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2016-07-18 19:05                   ` Jarkko Sakkinen
     [not found]     ` <1468544838-9990-3-git-send-email-apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2016-07-18 19:04       ` Jarkko Sakkinen
     [not found]         ` <20160718190421.GL31463-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-07-18 19:09           ` Jarkko Sakkinen
2016-07-18 19:13           ` Andrey Pronin

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=20160715193143.GA7052@apronin \
    --to=apronin@chromium.org \
    --cc=dianders@chromium.org \
    --cc=groeck@chromium.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    --cc=smbarber@chromium.org \
    --cc=tpmdd-devel@lists.sourceforge.net \
    --cc=tpmdd@selhorst.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).