* [PATCH v2 0/4] Refactor TPM event log code
@ 2018-04-12 10:13 Thiebaud Weksteen
2018-04-12 10:13 ` [PATCH v2 1/4] tpm: Add explicit endianness cast Thiebaud Weksteen
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Thiebaud Weksteen @ 2018-04-12 10:13 UTC (permalink / raw)
To: jarkko.sakkinen, nayna, linux-integrity, linux-kernel, tweek
Cc: linux-integrity, linux-kernel, Thiebaud Weksteen
This patchset implements the proposal from Jarkko Sakkinen [1]. I have
included the feedback from Nayna Jain about the function naming.
[1] https://lkml.kernel.org/r/20171024222148.gwnkj5vqsyj43qer@linux.intel.com
Changes since v1:
- Add Suggested-by
- Fix kbuild report
Thiebaud Weksteen (4):
tpm: Add explicit endianness cast
tpm: Move eventlog files to a subdirectory
tpm: Move shared eventlog functions to common.c
tpm: Move eventlog declarations to its own header
drivers/char/tpm/Makefile | 10 +-
.../{tpm_eventlog_acpi.c => eventlog/acpi.c} | 3 +-
drivers/char/tpm/eventlog/common.c | 195 +++++++++++++++++
drivers/char/tpm/eventlog/common.h | 35 +++
.../{tpm_eventlog_efi.c => eventlog/efi.c} | 3 +-
.../tpm/{tpm_eventlog_of.c => eventlog/of.c} | 7 +-
.../tpm/{tpm1_eventlog.c => eventlog/tpm1.c} | 200 ++----------------
.../tpm/{tpm2_eventlog.c => eventlog/tpm2.c} | 3 +-
drivers/char/tpm/tpm.h | 27 ---
9 files changed, 262 insertions(+), 221 deletions(-)
rename drivers/char/tpm/{tpm_eventlog_acpi.c => eventlog/acpi.c} (98%)
create mode 100644 drivers/char/tpm/eventlog/common.c
create mode 100644 drivers/char/tpm/eventlog/common.h
rename drivers/char/tpm/{tpm_eventlog_efi.c => eventlog/efi.c} (97%)
rename drivers/char/tpm/{tpm_eventlog_of.c => eventlog/of.c} (94%)
rename drivers/char/tpm/{tpm1_eventlog.c => eventlog/tpm1.c} (58%)
rename drivers/char/tpm/{tpm2_eventlog.c => eventlog/tpm2.c} (99%)
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-12 10:13 [PATCH v2 0/4] Refactor TPM event log code Thiebaud Weksteen @ 2018-04-12 10:13 ` Thiebaud Weksteen 2018-04-17 3:02 ` Jason Gunthorpe 2018-04-20 5:39 ` Jarkko Sakkinen 2018-04-12 10:13 ` [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory Thiebaud Weksteen ` (2 subsequent siblings) 3 siblings, 2 replies; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-12 10:13 UTC (permalink / raw) To: jarkko.sakkinen, nayna, linux-integrity, linux-kernel, tweek Cc: linux-integrity, linux-kernel, Thiebaud Weksteen Signed-off-by: Thiebaud Weksteen <tweek@google.com> --- drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_eventlog_of.c b/drivers/char/tpm/tpm_eventlog_of.c index 96fd5646f866..d74568d58a66 100644 --- a/drivers/char/tpm/tpm_eventlog_of.c +++ b/drivers/char/tpm/tpm_eventlog_of.c @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) * but physical tpm needs the conversion. */ if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0) { - size = be32_to_cpup(sizep); - base = be64_to_cpup(basep); + size = be32_to_cpup((__be32 *)sizep); + base = be64_to_cpup((__be64 *)basep); } else { size = *sizep; base = *basep; -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-12 10:13 ` [PATCH v2 1/4] tpm: Add explicit endianness cast Thiebaud Weksteen @ 2018-04-17 3:02 ` Jason Gunthorpe 2018-04-17 8:32 ` Thiebaud Weksteen 2018-04-20 5:39 ` Jarkko Sakkinen 1 sibling, 1 reply; 21+ messages in thread From: Jason Gunthorpe @ 2018-04-17 3:02 UTC (permalink / raw) To: Thiebaud Weksteen; +Cc: jarkko.sakkinen, nayna, linux-integrity, linux-kernel On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c b/drivers/char/tpm/tpm_eventlog_of.c > index 96fd5646f866..d74568d58a66 100644 > +++ b/drivers/char/tpm/tpm_eventlog_of.c > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > * but physical tpm needs the conversion. > */ > if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0) { > - size = be32_to_cpup(sizep); > - base = be64_to_cpup(basep); > + size = be32_to_cpup((__be32 *)sizep); > + base = be64_to_cpup((__be64 *)basep); Er, no.. change the definitions of sizep and basep to be __be Jason ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-17 3:02 ` Jason Gunthorpe @ 2018-04-17 8:32 ` Thiebaud Weksteen 2018-04-17 14:00 ` Jason Gunthorpe 0 siblings, 1 reply; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-17 8:32 UTC (permalink / raw) To: jgg; +Cc: Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c b/drivers/char/tpm/tpm_eventlog_of.c > > index 96fd5646f866..d74568d58a66 100644 > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > * but physical tpm needs the conversion. > > */ > > if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0) { > > - size = be32_to_cpup(sizep); > > - base = be64_to_cpup(basep); > > + size = be32_to_cpup((__be32 *)sizep); > > + base = be64_to_cpup((__be64 *)basep); > Er, no.. change the definitions of sizep and basep to be __be > Jason Please read the comment before the condition. sizep and basep may contain either little endian or big endian and this block is used to adjust that. Let me know if there is a better way for handling this. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-17 8:32 ` Thiebaud Weksteen @ 2018-04-17 14:00 ` Jason Gunthorpe 2018-04-19 13:09 ` Thiebaud Weksteen 0 siblings, 1 reply; 21+ messages in thread From: Jason Gunthorpe @ 2018-04-17 14:00 UTC (permalink / raw) To: Thiebaud Weksteen Cc: Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel On Tue, Apr 17, 2018 at 08:32:33AM +0000, Thiebaud Weksteen wrote: > On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c > b/drivers/char/tpm/tpm_eventlog_of.c > > > index 96fd5646f866..d74568d58a66 100644 > > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > > * but physical tpm needs the conversion. > > > */ > > > if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0) { > > > - size = be32_to_cpup(sizep); > > > - base = be64_to_cpup(basep); > > > + size = be32_to_cpup((__be32 *)sizep); > > > + base = be64_to_cpup((__be64 *)basep); > > > Er, no.. change the definitions of sizep and basep to be __be > > > Jason > > Please read the comment before the condition. sizep and > basep may contain either little endian or big endian and this block is used > to adjust that. Let me know if there is a better way for handling this. Well a cast like that will throw sparse warnings, you need __force at least Jason ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-17 14:00 ` Jason Gunthorpe @ 2018-04-19 13:09 ` Thiebaud Weksteen 2018-04-20 14:57 ` Jason Gunthorpe 0 siblings, 1 reply; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-19 13:09 UTC (permalink / raw) To: jgg; +Cc: Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel On Tue, Apr 17, 2018 at 4:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Tue, Apr 17, 2018 at 08:32:33AM +0000, Thiebaud Weksteen wrote: > > On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c > > b/drivers/char/tpm/tpm_eventlog_of.c > > > > index 96fd5646f866..d74568d58a66 100644 > > > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > > > * but physical tpm needs the conversion. > > > > */ > > > > if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0) { > > > > - size = be32_to_cpup(sizep); > > > > - base = be64_to_cpup(basep); > > > > + size = be32_to_cpup((__be32 *)sizep); > > > > + base = be64_to_cpup((__be64 *)basep); > > > > > Er, no.. change the definitions of sizep and basep to be __be > > > > > Jason > > > > Please read the comment before the condition. sizep and > > basep may contain either little endian or big endian and this block is used > > to adjust that. Let me know if there is a better way for handling this. > Well a cast like that will throw sparse warnings, you need __force at > least > Jason I don't think so. Since the variable is only defined as u32*, no specific warning is generated. I've used `make C=2 drivers/char/tpm/` with this patch applied and no new warning is being triggered. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-19 13:09 ` Thiebaud Weksteen @ 2018-04-20 14:57 ` Jason Gunthorpe 2018-04-23 9:22 ` Thiebaud Weksteen 0 siblings, 1 reply; 21+ messages in thread From: Jason Gunthorpe @ 2018-04-20 14:57 UTC (permalink / raw) To: Thiebaud Weksteen Cc: Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel On Thu, Apr 19, 2018 at 01:09:12PM +0000, Thiebaud Weksteen wrote: > On Tue, Apr 17, 2018 at 4:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > On Tue, Apr 17, 2018 at 08:32:33AM +0000, Thiebaud Weksteen wrote: > > > On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > > > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c > > > b/drivers/char/tpm/tpm_eventlog_of.c > > > > > index 96fd5646f866..d74568d58a66 100644 > > > > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > > > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > > > > * but physical tpm needs the conversion. > > > > > */ > > > > > if (of_property_match_string(np, "compatible", "IBM,vtpm") < > 0) { > > > > > - size = be32_to_cpup(sizep); > > > > > - base = be64_to_cpup(basep); > > > > > + size = be32_to_cpup((__be32 *)sizep); > > > > > + base = be64_to_cpup((__be64 *)basep); > > > > > > > Er, no.. change the definitions of sizep and basep to be __be > > > > > > > Jason > > > > > > Please read the comment before the condition. sizep and > > > basep may contain either little endian or big endian and this block is > used > > > to adjust that. Let me know if there is a better way for handling this. > > > Well a cast like that will throw sparse warnings, you need __force at > > least > > I don't think so. Since the variable is only defined as u32*, no specific > warning is generated. I've used `make C=2 drivers/char/tpm/` with this > patch applied and no new warning is being triggered. I'm surprised to hear you say that.. Sparse is supposed to require force on all cast that change the annotation, and there are many examples in the kernel that have force in that case. Jason ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-20 14:57 ` Jason Gunthorpe @ 2018-04-23 9:22 ` Thiebaud Weksteen 2018-04-23 10:06 ` Luc Van Oostenryck 0 siblings, 1 reply; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-23 9:22 UTC (permalink / raw) To: jgg, Christopher Li Cc: Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel, linux-sparse On Fri, Apr 20, 2018 at 4:57 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Thu, Apr 19, 2018 at 01:09:12PM +0000, Thiebaud Weksteen wrote: > > On Tue, Apr 17, 2018 at 4:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > On Tue, Apr 17, 2018 at 08:32:33AM +0000, Thiebaud Weksteen wrote: > > > > On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > > > > > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > > > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > > > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c > > > > b/drivers/char/tpm/tpm_eventlog_of.c > > > > > > index 96fd5646f866..d74568d58a66 100644 > > > > > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > > > > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > > > > > * but physical tpm needs the conversion. > > > > > > */ > > > > > > if (of_property_match_string(np, "compatible", "IBM,vtpm") < > > 0) { > > > > > > - size = be32_to_cpup(sizep); > > > > > > - base = be64_to_cpup(basep); > > > > > > + size = be32_to_cpup((__be32 *)sizep); > > > > > > + base = be64_to_cpup((__be64 *)basep); > > > > > > > > > Er, no.. change the definitions of sizep and basep to be __be > > > > > > > > > Jason > > > > > > > > Please read the comment before the condition. sizep and > > > > basep may contain either little endian or big endian and this block is > > used > > > > to adjust that. Let me know if there is a better way for handling this. > > > > > Well a cast like that will throw sparse warnings, you need __force at > > > least > > > > I don't think so. Since the variable is only defined as u32*, no specific > > warning is generated. I've used `make C=2 drivers/char/tpm/` with this > > patch applied and no new warning is being triggered. > I'm surprised to hear you say that.. > Sparse is supposed to require force on all cast that change the > annotation, and there are many examples in the kernel that have force > in that case. > Jason +linux-sparse@vger.kernel.org and sparse@chrisli.org for a sanity check. If you look at the man page of sparse, under the bitwise option, it states: "Sparse will warn on [...] any conversion of one restricted type into another, except via a cast that includes __attribute__((force)).". In our case, it is a conversion from unrestricted to restricted which does not fall in this category. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-23 9:22 ` Thiebaud Weksteen @ 2018-04-23 10:06 ` Luc Van Oostenryck 2018-04-24 9:57 ` Thiebaud Weksteen 0 siblings, 1 reply; 21+ messages in thread From: Luc Van Oostenryck @ 2018-04-23 10:06 UTC (permalink / raw) To: Thiebaud Weksteen Cc: jgg, Christopher Li, Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel, linux-sparse, Josh Triplett On Mon, Apr 23, 2018 at 09:22:06AM +0000, Thiebaud Weksteen wrote: > On Fri, Apr 20, 2018 at 4:57 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > On Thu, Apr 19, 2018 at 01:09:12PM +0000, Thiebaud Weksteen wrote: > > > On Tue, Apr 17, 2018 at 4:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > > > On Tue, Apr 17, 2018 at 08:32:33AM +0000, Thiebaud Weksteen wrote: > > > > > On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> > wrote: > > > > > > > > > > > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > > > > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > > > > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c > > > > > b/drivers/char/tpm/tpm_eventlog_of.c > > > > > > > index 96fd5646f866..d74568d58a66 100644 > > > > > > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > > > > > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > > > > > > * but physical tpm needs the conversion. > > > > > > > */ > > > > > > > if (of_property_match_string(np, "compatible", > "IBM,vtpm") < > > > 0) { > > > > > > > - size = be32_to_cpup(sizep); > > > > > > > - base = be64_to_cpup(basep); > > > > > > > + size = be32_to_cpup((__be32 *)sizep); > > > > > > > + base = be64_to_cpup((__be64 *)basep); > > > > > > > > > > > Er, no.. change the definitions of sizep and basep to be __be > > > > > > > > > > > Jason > > > > > > > > > > Please read the comment before the condition. sizep and > > > > > basep may contain either little endian or big endian and this block > is > > > used > > > > > to adjust that. Let me know if there is a better way for handling > this. > > > > > > > Well a cast like that will throw sparse warnings, you need __force at > > > > least > > > > > > I don't think so. Since the variable is only defined as u32*, no > specific > > > warning is generated. I've used `make C=2 drivers/char/tpm/` with this > > > patch applied and no new warning is being triggered. Interesting. > > I'm surprised to hear you say that.. Same for me. > > Sparse is supposed to require force on all cast that change the > > annotation, and there are many examples in the kernel that have force > > in that case. Yes, sparse is supposed to warn in such cases and the __force is there to quiets the warning when it is known that the cast is in fact correct. > +linux-sparse@vger.kernel.org and sparse@chrisli.org for a sanity check. > > If you look at the man page of sparse, under the bitwise option, it states: > "Sparse will warn on [...] any conversion of one restricted type into > another, except via a cast that includes __attribute__((force)).". In our > case, it is a conversion from unrestricted to restricted which does not > fall in this category. The man page is not very clear here. It must be understood in the context where each use of '__bitwise' will create a new *distinct* type. Given this and the normal type checking, sparse should warn "on any conversion of one restricted type into another *or* between a restricted and the corresponding plain/unrestricted type" (or consider that an 'unrestricted type' is 'a restricted type with no restriction', which is, I think, what was meant here). The fact that sparse doesn't warn in your case is clearly a bug in sparse's type checking. Regards, -- Luc Van Oostenryck ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-23 10:06 ` Luc Van Oostenryck @ 2018-04-24 9:57 ` Thiebaud Weksteen 0 siblings, 0 replies; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-24 9:57 UTC (permalink / raw) To: luc.vanoostenryck Cc: jgg, Christopher Li, Jarkko Sakkinen, Nayna Jain, linux-integrity, linux-kernel, linux-sparse, josh On Mon, Apr 23, 2018 at 12:06 PM Luc Van Oostenryck < luc.vanoostenryck@gmail.com> wrote: > On Mon, Apr 23, 2018 at 09:22:06AM +0000, Thiebaud Weksteen wrote: > > On Fri, Apr 20, 2018 at 4:57 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > On Thu, Apr 19, 2018 at 01:09:12PM +0000, Thiebaud Weksteen wrote: > > > > On Tue, Apr 17, 2018 at 4:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > > > > > > On Tue, Apr 17, 2018 at 08:32:33AM +0000, Thiebaud Weksteen wrote: > > > > > > On Tue, Apr 17, 2018 at 5:02 AM Jason Gunthorpe <jgg@ziepe.ca> > > wrote: > > > > > > > > > > > > > On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > > > > > > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > > > > > > drivers/char/tpm/tpm_eventlog_of.c | 4 ++-- > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_eventlog_of.c > > > > > > b/drivers/char/tpm/tpm_eventlog_of.c > > > > > > > > index 96fd5646f866..d74568d58a66 100644 > > > > > > > > +++ b/drivers/char/tpm/tpm_eventlog_of.c > > > > > > > > @@ -56,8 +56,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > > > > > > > > * but physical tpm needs the conversion. > > > > > > > > */ > > > > > > > > if (of_property_match_string(np, "compatible", > > "IBM,vtpm") < > > > > 0) { > > > > > > > > - size = be32_to_cpup(sizep); > > > > > > > > - base = be64_to_cpup(basep); > > > > > > > > + size = be32_to_cpup((__be32 *)sizep); > > > > > > > > + base = be64_to_cpup((__be64 *)basep); > > > > > > > > > > > > > Er, no.. change the definitions of sizep and basep to be __be > > > > > > > > > > > > > Jason > > > > > > > > > > > > Please read the comment before the condition. sizep and > > > > > > basep may contain either little endian or big endian and this block > > is > > > > used > > > > > > to adjust that. Let me know if there is a better way for handling > > this. > > > > > > > > > Well a cast like that will throw sparse warnings, you need __force at > > > > > least > > > > > > > > I don't think so. Since the variable is only defined as u32*, no > > specific > > > > warning is generated. I've used `make C=2 drivers/char/tpm/` with this > > > > patch applied and no new warning is being triggered. > Interesting. > > > I'm surprised to hear you say that.. > Same for me. > > > Sparse is supposed to require force on all cast that change the > > > annotation, and there are many examples in the kernel that have force > > > in that case. > Yes, sparse is supposed to warn in such cases and the __force is there > to quiets the warning when it is known that the cast is in fact correct. > > +linux-sparse@vger.kernel.org and sparse@chrisli.org for a sanity check. > > > > If you look at the man page of sparse, under the bitwise option, it states: > > "Sparse will warn on [...] any conversion of one restricted type into > > another, except via a cast that includes __attribute__((force)).". In our > > case, it is a conversion from unrestricted to restricted which does not > > fall in this category. > The man page is not very clear here. It must be understood in the context > where each use of '__bitwise' will create a new *distinct* type. > Given this and the normal type checking, sparse should warn > "on any conversion of one restricted type into another *or* between a > restricted and the corresponding plain/unrestricted type" (or consider > that an 'unrestricted type' is 'a restricted type with no restriction', > which is, I think, what was meant here). Thanks for the explanation, that make sense. I believe the issue happens when dealing with restricted pointer types more than regular types. Also, this is not new and has probably been going on for the last 13 years. For instance, 81179bb6a54c2c626b4cbcc084ca974bb2d7f2a3 explicitly removed the __force attribute. I'll send a validation patch for sparse and update this patch. > The fact that sparse doesn't warn in your case is clearly a bug in > sparse's type checking. > Regards, > -- Luc Van Oostenryck ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/4] tpm: Add explicit endianness cast 2018-04-12 10:13 ` [PATCH v2 1/4] tpm: Add explicit endianness cast Thiebaud Weksteen 2018-04-17 3:02 ` Jason Gunthorpe @ 2018-04-20 5:39 ` Jarkko Sakkinen 1 sibling, 0 replies; 21+ messages in thread From: Jarkko Sakkinen @ 2018-04-20 5:39 UTC (permalink / raw) To: Thiebaud Weksteen; +Cc: nayna, linux-integrity, linux-kernel On Thu, Apr 12, 2018 at 12:13:47PM +0200, Thiebaud Weksteen wrote: > Signed-off-by: Thiebaud Weksteen <tweek@google.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory 2018-04-12 10:13 [PATCH v2 0/4] Refactor TPM event log code Thiebaud Weksteen 2018-04-12 10:13 ` [PATCH v2 1/4] tpm: Add explicit endianness cast Thiebaud Weksteen @ 2018-04-12 10:13 ` Thiebaud Weksteen 2018-04-20 5:39 ` Jarkko Sakkinen 2018-04-12 10:13 ` [PATCH v2 3/4] tpm: Move shared eventlog functions to common.c Thiebaud Weksteen 2018-04-12 10:13 ` [PATCH v2 4/4] tpm: Move eventlog declarations to its own header Thiebaud Weksteen 3 siblings, 1 reply; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-12 10:13 UTC (permalink / raw) To: jarkko.sakkinen, nayna, linux-integrity, linux-kernel, tweek Cc: linux-integrity, linux-kernel, Thiebaud Weksteen Signed-off-by: Thiebaud Weksteen <tweek@google.com> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- drivers/char/tpm/Makefile | 8 ++++---- drivers/char/tpm/{tpm_eventlog_acpi.c => eventlog/acpi.c} | 2 +- drivers/char/tpm/{tpm_eventlog_efi.c => eventlog/efi.c} | 2 +- drivers/char/tpm/{tpm_eventlog_of.c => eventlog/of.c} | 2 +- drivers/char/tpm/{tpm1_eventlog.c => eventlog/tpm1.c} | 2 +- drivers/char/tpm/{tpm2_eventlog.c => eventlog/tpm2.c} | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) rename drivers/char/tpm/{tpm_eventlog_acpi.c => eventlog/acpi.c} (99%) rename drivers/char/tpm/{tpm_eventlog_efi.c => eventlog/efi.c} (98%) rename drivers/char/tpm/{tpm_eventlog_of.c => eventlog/of.c} (99%) rename drivers/char/tpm/{tpm1_eventlog.c => eventlog/tpm1.c} (99%) rename drivers/char/tpm/{tpm2_eventlog.c => eventlog/tpm2.c} (99%) diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile index acd758381c58..5dcf5bd35a3d 100644 --- a/drivers/char/tpm/Makefile +++ b/drivers/char/tpm/Makefile @@ -4,11 +4,11 @@ # obj-$(CONFIG_TCG_TPM) += tpm.o tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \ - tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \ + tpm-dev-common.o tpmrm-dev.o eventlog/tpm1.o eventlog/tpm2.o \ tpm2-space.o -tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o -tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o -tpm-$(CONFIG_OF) += tpm_eventlog_of.o +tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o +tpm-$(CONFIG_EFI) += eventlog/efi.o +tpm-$(CONFIG_OF) += eventlog/of.o obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o obj-$(CONFIG_TCG_TIS) += tpm_tis.o obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o diff --git a/drivers/char/tpm/tpm_eventlog_acpi.c b/drivers/char/tpm/eventlog/acpi.c similarity index 99% rename from drivers/char/tpm/tpm_eventlog_acpi.c rename to drivers/char/tpm/eventlog/acpi.c index 66f19e93c216..8476be2e9526 100644 --- a/drivers/char/tpm/tpm_eventlog_acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -27,7 +27,7 @@ #include <linux/acpi.h> #include <linux/tpm_eventlog.h> -#include "tpm.h" +#include "../tpm.h" struct acpi_tcpa { struct acpi_table_header hdr; diff --git a/drivers/char/tpm/tpm_eventlog_efi.c b/drivers/char/tpm/eventlog/efi.c similarity index 98% rename from drivers/char/tpm/tpm_eventlog_efi.c rename to drivers/char/tpm/eventlog/efi.c index e3f9ffd341d2..e1593c5271a4 100644 --- a/drivers/char/tpm/tpm_eventlog_efi.c +++ b/drivers/char/tpm/eventlog/efi.c @@ -14,7 +14,7 @@ #include <linux/efi.h> #include <linux/tpm_eventlog.h> -#include "tpm.h" +#include "../tpm.h" /* read binary bios log from EFI configuration table */ int tpm_read_log_efi(struct tpm_chip *chip) diff --git a/drivers/char/tpm/tpm_eventlog_of.c b/drivers/char/tpm/eventlog/of.c similarity index 99% rename from drivers/char/tpm/tpm_eventlog_of.c rename to drivers/char/tpm/eventlog/of.c index d74568d58a66..d98c184d5c13 100644 --- a/drivers/char/tpm/tpm_eventlog_of.c +++ b/drivers/char/tpm/eventlog/of.c @@ -19,7 +19,7 @@ #include <linux/of.h> #include <linux/tpm_eventlog.h> -#include "tpm.h" +#include "../tpm.h" int tpm_read_log_of(struct tpm_chip *chip) { diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/eventlog/tpm1.c similarity index 99% rename from drivers/char/tpm/tpm1_eventlog.c rename to drivers/char/tpm/eventlog/tpm1.c index add798bd69d0..d6aea3ca950e 100644 --- a/drivers/char/tpm/tpm1_eventlog.c +++ b/drivers/char/tpm/eventlog/tpm1.c @@ -28,7 +28,7 @@ #include <linux/slab.h> #include <linux/tpm_eventlog.h> -#include "tpm.h" +#include "../tpm.h" static const char* tcpa_event_type_strings[] = { diff --git a/drivers/char/tpm/tpm2_eventlog.c b/drivers/char/tpm/eventlog/tpm2.c similarity index 99% rename from drivers/char/tpm/tpm2_eventlog.c rename to drivers/char/tpm/eventlog/tpm2.c index 1ce4411292ba..f0723fa9ae14 100644 --- a/drivers/char/tpm/tpm2_eventlog.c +++ b/drivers/char/tpm/eventlog/tpm2.c @@ -23,7 +23,7 @@ #include <linux/slab.h> #include <linux/tpm_eventlog.h> -#include "tpm.h" +#include "../tpm.h" /* * calc_tpm2_event_size() - calculate the event size, where event -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory 2018-04-12 10:13 ` [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory Thiebaud Weksteen @ 2018-04-20 5:39 ` Jarkko Sakkinen 2018-05-07 20:36 ` Mimi Zohar 0 siblings, 1 reply; 21+ messages in thread From: Jarkko Sakkinen @ 2018-04-20 5:39 UTC (permalink / raw) To: Thiebaud Weksteen; +Cc: nayna, linux-integrity, linux-kernel On Thu, Apr 12, 2018 at 12:13:48PM +0200, Thiebaud Weksteen wrote: > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory 2018-04-20 5:39 ` Jarkko Sakkinen @ 2018-05-07 20:36 ` Mimi Zohar 2018-05-09 14:51 ` Petr Vorel 2018-05-10 1:39 ` Jarkko Sakkinen 0 siblings, 2 replies; 21+ messages in thread From: Mimi Zohar @ 2018-05-07 20:36 UTC (permalink / raw) To: Jarkko Sakkinen, Thiebaud Weksteen Cc: nayna, linux-integrity, linux-kernel, ltp, Petr Vorel [Cc'ing Petr Vorel and the ltp mailing list] Hi Jarrko, On Fri, 2018-04-20 at 08:39 +0300, Jarkko Sakkinen wrote: > On Thu, Apr 12, 2018 at 12:13:48PM +0200, Thiebaud Weksteen wrote: > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> I just noticed this is queued in your next branch. Petr Vorel has been updating the IMA LTP tests. One of those IMA LTP tests includes walking the TPM binary_runtime_measurements in order to calculate the IMA boot-aggregate. The IMA boot-aggregate is the first measurement in the IMA measurement list. Mimi ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory 2018-05-07 20:36 ` Mimi Zohar @ 2018-05-09 14:51 ` Petr Vorel 2018-05-11 1:51 ` Mimi Zohar 2018-05-10 1:39 ` Jarkko Sakkinen 1 sibling, 1 reply; 21+ messages in thread From: Petr Vorel @ 2018-05-09 14:51 UTC (permalink / raw) To: Mimi Zohar Cc: Jarkko Sakkinen, Thiebaud Weksteen, nayna, linux-integrity, linux-kernel, ltp Hi Mimi, > [Cc'ing Petr Vorel and the ltp mailing list] > Hi Jarrko, > On Fri, 2018-04-20 at 08:39 +0300, Jarkko Sakkinen wrote: > > On Thu, Apr 12, 2018 at 12:13:48PM +0200, Thiebaud Weksteen wrote: > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > I just noticed this is queued in your next branch. > Petr Vorel has been updating the IMA LTP tests. One of those IMA LTP > tests includes walking the TPM binary_runtime_measurements in order to > calculate the IMA boot-aggregate. The IMA boot-aggregate is the first > measurement in the IMA measurement list. Did you meant that this commit ([2]) in linux-tpmdd/next changed location of binary_runtime_measurements in sysfs? IMHO these commits ([1], [2], [3]) just put source code into eventlog/ subdirectory: > Mimi Kind regards, Petr [1] 75d647f5de69 tpm: Move eventlog declarations to its own header [2] 9b01b5356629 tpm: Move shared eventlog functions to common.c [3] 0bfb23746052 tpm: Move eventlog files to a subdirectory ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory 2018-05-09 14:51 ` Petr Vorel @ 2018-05-11 1:51 ` Mimi Zohar 0 siblings, 0 replies; 21+ messages in thread From: Mimi Zohar @ 2018-05-11 1:51 UTC (permalink / raw) To: Petr Vorel Cc: Jarkko Sakkinen, Thiebaud Weksteen, nayna, linux-integrity, linux-kernel, ltp On Wed, 2018-05-09 at 16:51 +0200, Petr Vorel wrote: > Hi Mimi, > > > [Cc'ing Petr Vorel and the ltp mailing list] > > > Hi Jarrko, > > > On Fri, 2018-04-20 at 08:39 +0300, Jarkko Sakkinen wrote: > > > On Thu, Apr 12, 2018 at 12:13:48PM +0200, Thiebaud Weksteen wrote: > > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > I just noticed this is queued in your next branch. > > > Petr Vorel has been updating the IMA LTP tests. One of those IMA LTP > > tests includes walking the TPM binary_runtime_measurements in order to > > calculate the IMA boot-aggregate. The IMA boot-aggregate is the first > > measurement in the IMA measurement list. > > Did you meant that this commit ([2]) in linux-tpmdd/next changed location of > binary_runtime_measurements in sysfs? > > IMHO these commits ([1], [2], [3]) just put source code into eventlog/ subdirectory: I mistakenly thought the eventlog itself was being moved, not the source code. thanks, Mimi > > Kind regards, > Petr > > [1] 75d647f5de69 tpm: Move eventlog declarations to its own header > [2] 9b01b5356629 tpm: Move shared eventlog functions to common.c > [3] 0bfb23746052 tpm: Move eventlog files to a subdirectory > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory 2018-05-07 20:36 ` Mimi Zohar 2018-05-09 14:51 ` Petr Vorel @ 2018-05-10 1:39 ` Jarkko Sakkinen 1 sibling, 0 replies; 21+ messages in thread From: Jarkko Sakkinen @ 2018-05-10 1:39 UTC (permalink / raw) To: Mimi Zohar Cc: Thiebaud Weksteen, nayna, linux-integrity, linux-kernel, ltp, Petr Vorel On Mon, May 07, 2018 at 04:36:37PM -0400, Mimi Zohar wrote: > [Cc'ing Petr Vorel and the ltp mailing list] > > Hi Jarrko, > > On Fri, 2018-04-20 at 08:39 +0300, Jarkko Sakkinen wrote: > > On Thu, Apr 12, 2018 at 12:13:48PM +0200, Thiebaud Weksteen wrote: > > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > > > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > I just noticed this is queued in your next branch. > > Petr Vorel has been updating the IMA LTP tests. One of those IMA LTP > tests includes walking the TPM binary_runtime_measurements in order to > calculate the IMA boot-aggregate. The IMA boot-aggregate is the first > measurement in the IMA measurement list. > > Mimi Sorry, not following how these connect directly? /Jarkko ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/4] tpm: Move shared eventlog functions to common.c 2018-04-12 10:13 [PATCH v2 0/4] Refactor TPM event log code Thiebaud Weksteen 2018-04-12 10:13 ` [PATCH v2 1/4] tpm: Add explicit endianness cast Thiebaud Weksteen 2018-04-12 10:13 ` [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory Thiebaud Weksteen @ 2018-04-12 10:13 ` Thiebaud Weksteen 2018-04-20 5:40 ` Jarkko Sakkinen 2018-04-12 10:13 ` [PATCH v2 4/4] tpm: Move eventlog declarations to its own header Thiebaud Weksteen 3 siblings, 1 reply; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-12 10:13 UTC (permalink / raw) To: jarkko.sakkinen, nayna, linux-integrity, linux-kernel, tweek Cc: linux-integrity, linux-kernel, Thiebaud Weksteen Functions and structures specific to TPM1 are renamed from tpm* to tpm1*. Signed-off-by: Thiebaud Weksteen <tweek@google.com> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- drivers/char/tpm/Makefile | 4 +- drivers/char/tpm/eventlog/common.c | 195 ++++++++++++++++++++++++++++ drivers/char/tpm/eventlog/tpm1.c | 197 +++-------------------------- drivers/char/tpm/tpm.h | 2 + 4 files changed, 214 insertions(+), 184 deletions(-) create mode 100644 drivers/char/tpm/eventlog/common.c diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile index 5dcf5bd35a3d..4e9c33ca1f8f 100644 --- a/drivers/char/tpm/Makefile +++ b/drivers/char/tpm/Makefile @@ -4,8 +4,8 @@ # obj-$(CONFIG_TCG_TPM) += tpm.o tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \ - tpm-dev-common.o tpmrm-dev.o eventlog/tpm1.o eventlog/tpm2.o \ - tpm2-space.o + tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \ + eventlog/tpm2.o tpm2-space.o tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o tpm-$(CONFIG_EFI) += eventlog/efi.o tpm-$(CONFIG_OF) += eventlog/of.o diff --git a/drivers/char/tpm/eventlog/common.c b/drivers/char/tpm/eventlog/common.c new file mode 100644 index 000000000000..54934b5a1566 --- /dev/null +++ b/drivers/char/tpm/eventlog/common.c @@ -0,0 +1,195 @@ +/* + * Copyright (C) 2005, 2012 IBM Corporation + * + * Authors: + * Kent Yoder <key@linux.vnet.ibm.com> + * Seiji Munetoh <munetoh@jp.ibm.com> + * Stefan Berger <stefanb@us.ibm.com> + * Reiner Sailer <sailer@watson.ibm.com> + * Kylene Hall <kjhall@us.ibm.com> + * Nayna Jain <nayna@linux.vnet.ibm.com> + * + * Access to the event log created by a system's firmware / BIOS + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + * + */ + +#include <linux/seq_file.h> +#include <linux/fs.h> +#include <linux/security.h> +#include <linux/module.h> +#include <linux/tpm_eventlog.h> + +#include "../tpm.h" + + +static int tpm_bios_measurements_open(struct inode *inode, + struct file *file) +{ + int err; + struct seq_file *seq; + struct tpm_chip_seqops *chip_seqops; + const struct seq_operations *seqops; + struct tpm_chip *chip; + + inode_lock(inode); + if (!inode->i_private) { + inode_unlock(inode); + return -ENODEV; + } + chip_seqops = (struct tpm_chip_seqops *)inode->i_private; + seqops = chip_seqops->seqops; + chip = chip_seqops->chip; + get_device(&chip->dev); + inode_unlock(inode); + + /* now register seq file */ + err = seq_open(file, seqops); + if (!err) { + seq = file->private_data; + seq->private = chip; + } + + return err; +} + +static int tpm_bios_measurements_release(struct inode *inode, + struct file *file) +{ + struct seq_file *seq = (struct seq_file *)file->private_data; + struct tpm_chip *chip = (struct tpm_chip *)seq->private; + + put_device(&chip->dev); + + return seq_release(inode, file); +} + +static const struct file_operations tpm_bios_measurements_ops = { + .owner = THIS_MODULE, + .open = tpm_bios_measurements_open, + .read = seq_read, + .llseek = seq_lseek, + .release = tpm_bios_measurements_release, +}; + +static int tpm_read_log(struct tpm_chip *chip) +{ + int rc; + + if (chip->log.bios_event_log != NULL) { + dev_dbg(&chip->dev, + "%s: ERROR - event log already initialized\n", + __func__); + return -EFAULT; + } + + rc = tpm_read_log_acpi(chip); + if (rc != -ENODEV) + return rc; + + rc = tpm_read_log_efi(chip); + if (rc != -ENODEV) + return rc; + + return tpm_read_log_of(chip); +} + +/* + * tpm_bios_log_setup() - Read the event log from the firmware + * @chip: TPM chip to use. + * + * If an event log is found then the securityfs files are setup to + * export it to userspace, otherwise nothing is done. + * + * Returns -ENODEV if the firmware has no event log or securityfs is not + * supported. + */ +int tpm_bios_log_setup(struct tpm_chip *chip) +{ + const char *name = dev_name(&chip->dev); + unsigned int cnt; + int log_version; + int rc = 0; + + rc = tpm_read_log(chip); + if (rc < 0) + return rc; + log_version = rc; + + cnt = 0; + chip->bios_dir[cnt] = securityfs_create_dir(name, NULL); + /* NOTE: securityfs_create_dir can return ENODEV if securityfs is + * compiled out. The caller should ignore the ENODEV return code. + */ + if (IS_ERR(chip->bios_dir[cnt])) + goto err; + cnt++; + + chip->bin_log_seqops.chip = chip; + if (log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2) + chip->bin_log_seqops.seqops = + &tpm2_binary_b_measurements_seqops; + else + chip->bin_log_seqops.seqops = + &tpm1_binary_b_measurements_seqops; + + + chip->bios_dir[cnt] = + securityfs_create_file("binary_bios_measurements", + 0440, chip->bios_dir[0], + (void *)&chip->bin_log_seqops, + &tpm_bios_measurements_ops); + if (IS_ERR(chip->bios_dir[cnt])) + goto err; + cnt++; + + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { + + chip->ascii_log_seqops.chip = chip; + chip->ascii_log_seqops.seqops = + &tpm1_ascii_b_measurements_seqops; + + chip->bios_dir[cnt] = + securityfs_create_file("ascii_bios_measurements", + 0440, chip->bios_dir[0], + (void *)&chip->ascii_log_seqops, + &tpm_bios_measurements_ops); + if (IS_ERR(chip->bios_dir[cnt])) + goto err; + cnt++; + } + + return 0; + +err: + rc = PTR_ERR(chip->bios_dir[cnt]); + chip->bios_dir[cnt] = NULL; + tpm_bios_log_teardown(chip); + return rc; +} + +void tpm_bios_log_teardown(struct tpm_chip *chip) +{ + int i; + struct inode *inode; + + /* securityfs_remove currently doesn't take care of handling sync + * between removal and opening of pseudo files. To handle this, a + * workaround is added by making i_private = NULL here during removal + * and to check it during open(), both within inode_lock()/unlock(). + * This design ensures that open() either safely gets kref or fails. + */ + for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) { + if (chip->bios_dir[i]) { + inode = d_inode(chip->bios_dir[i]); + inode_lock(inode); + inode->i_private = NULL; + inode_unlock(inode); + securityfs_remove(chip->bios_dir[i]); + } + } +} diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c index d6aea3ca950e..8f30316e9bb6 100644 --- a/drivers/char/tpm/eventlog/tpm1.c +++ b/drivers/char/tpm/eventlog/tpm1.c @@ -71,7 +71,7 @@ static const char* tcpa_pc_event_id_strings[] = { }; /* returns pointer to start of pos. entry of tcg log */ -static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) +static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos) { loff_t i; struct tpm_chip *chip = m->private; @@ -118,7 +118,7 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) return addr; } -static void *tpm_bios_measurements_next(struct seq_file *m, void *v, +static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, loff_t *pos) { struct tcpa_event *event = v; @@ -149,7 +149,7 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, return v; } -static void tpm_bios_measurements_stop(struct seq_file *m, void *v) +static void tpm1_bios_measurements_stop(struct seq_file *m, void *v) { } @@ -232,7 +232,7 @@ static int get_event_name(char *dest, struct tcpa_event *event, } -static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v) +static int tpm1_binary_bios_measurements_show(struct seq_file *m, void *v) { struct tcpa_event *event = v; struct tcpa_event temp_event; @@ -261,18 +261,7 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v) } -static int tpm_bios_measurements_release(struct inode *inode, - struct file *file) -{ - struct seq_file *seq = (struct seq_file *)file->private_data; - struct tpm_chip *chip = (struct tpm_chip *)seq->private; - - put_device(&chip->dev); - - return seq_release(inode, file); -} - -static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v) +static int tpm1_ascii_bios_measurements_show(struct seq_file *m, void *v) { int len = 0; char *eventname; @@ -305,172 +294,16 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v) return 0; } -static const struct seq_operations tpm_ascii_b_measurements_seqops = { - .start = tpm_bios_measurements_start, - .next = tpm_bios_measurements_next, - .stop = tpm_bios_measurements_stop, - .show = tpm_ascii_bios_measurements_show, +const struct seq_operations tpm1_ascii_b_measurements_seqops = { + .start = tpm1_bios_measurements_start, + .next = tpm1_bios_measurements_next, + .stop = tpm1_bios_measurements_stop, + .show = tpm1_ascii_bios_measurements_show, }; -static const struct seq_operations tpm_binary_b_measurements_seqops = { - .start = tpm_bios_measurements_start, - .next = tpm_bios_measurements_next, - .stop = tpm_bios_measurements_stop, - .show = tpm_binary_bios_measurements_show, -}; - -static int tpm_bios_measurements_open(struct inode *inode, - struct file *file) -{ - int err; - struct seq_file *seq; - struct tpm_chip_seqops *chip_seqops; - const struct seq_operations *seqops; - struct tpm_chip *chip; - - inode_lock(inode); - if (!inode->i_private) { - inode_unlock(inode); - return -ENODEV; - } - chip_seqops = (struct tpm_chip_seqops *)inode->i_private; - seqops = chip_seqops->seqops; - chip = chip_seqops->chip; - get_device(&chip->dev); - inode_unlock(inode); - - /* now register seq file */ - err = seq_open(file, seqops); - if (!err) { - seq = file->private_data; - seq->private = chip; - } - - return err; -} - -static const struct file_operations tpm_bios_measurements_ops = { - .owner = THIS_MODULE, - .open = tpm_bios_measurements_open, - .read = seq_read, - .llseek = seq_lseek, - .release = tpm_bios_measurements_release, +const struct seq_operations tpm1_binary_b_measurements_seqops = { + .start = tpm1_bios_measurements_start, + .next = tpm1_bios_measurements_next, + .stop = tpm1_bios_measurements_stop, + .show = tpm1_binary_bios_measurements_show, }; - -static int tpm_read_log(struct tpm_chip *chip) -{ - int rc; - - if (chip->log.bios_event_log != NULL) { - dev_dbg(&chip->dev, - "%s: ERROR - event log already initialized\n", - __func__); - return -EFAULT; - } - - rc = tpm_read_log_acpi(chip); - if (rc != -ENODEV) - return rc; - - rc = tpm_read_log_efi(chip); - if (rc != -ENODEV) - return rc; - - return tpm_read_log_of(chip); -} - -/* - * tpm_bios_log_setup() - Read the event log from the firmware - * @chip: TPM chip to use. - * - * If an event log is found then the securityfs files are setup to - * export it to userspace, otherwise nothing is done. - * - * Returns -ENODEV if the firmware has no event log or securityfs is not - * supported. - */ -int tpm_bios_log_setup(struct tpm_chip *chip) -{ - const char *name = dev_name(&chip->dev); - unsigned int cnt; - int log_version; - int rc = 0; - - rc = tpm_read_log(chip); - if (rc < 0) - return rc; - log_version = rc; - - cnt = 0; - chip->bios_dir[cnt] = securityfs_create_dir(name, NULL); - /* NOTE: securityfs_create_dir can return ENODEV if securityfs is - * compiled out. The caller should ignore the ENODEV return code. - */ - if (IS_ERR(chip->bios_dir[cnt])) - goto err; - cnt++; - - chip->bin_log_seqops.chip = chip; - if (log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2) - chip->bin_log_seqops.seqops = - &tpm2_binary_b_measurements_seqops; - else - chip->bin_log_seqops.seqops = - &tpm_binary_b_measurements_seqops; - - - chip->bios_dir[cnt] = - securityfs_create_file("binary_bios_measurements", - 0440, chip->bios_dir[0], - (void *)&chip->bin_log_seqops, - &tpm_bios_measurements_ops); - if (IS_ERR(chip->bios_dir[cnt])) - goto err; - cnt++; - - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { - - chip->ascii_log_seqops.chip = chip; - chip->ascii_log_seqops.seqops = - &tpm_ascii_b_measurements_seqops; - - chip->bios_dir[cnt] = - securityfs_create_file("ascii_bios_measurements", - 0440, chip->bios_dir[0], - (void *)&chip->ascii_log_seqops, - &tpm_bios_measurements_ops); - if (IS_ERR(chip->bios_dir[cnt])) - goto err; - cnt++; - } - - return 0; - -err: - rc = PTR_ERR(chip->bios_dir[cnt]); - chip->bios_dir[cnt] = NULL; - tpm_bios_log_teardown(chip); - return rc; -} - -void tpm_bios_log_teardown(struct tpm_chip *chip) -{ - int i; - struct inode *inode; - - /* securityfs_remove currently doesn't take care of handling sync - * between removal and opening of pseudo files. To handle this, a - * workaround is added by making i_private = NULL here during removal - * and to check it during open(), both within inode_lock()/unlock(). - * This design ensures that open() either safely gets kref or fails. - */ - for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) { - if (chip->bios_dir[i]) { - inode = d_inode(chip->bios_dir[i]); - inode_lock(inode); - inode->i_private = NULL; - inode_unlock(inode); - securityfs_remove(chip->bios_dir[i]); - } - } -} diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index f895fba4e20d..a583c5001904 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -580,6 +580,8 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc, int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc, u8 *buf, size_t *bufsiz); +extern const struct seq_operations tpm1_ascii_b_measurements_seqops; +extern const struct seq_operations tpm1_binary_b_measurements_seqops; extern const struct seq_operations tpm2_binary_b_measurements_seqops; #if defined(CONFIG_ACPI) -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/4] tpm: Move shared eventlog functions to common.c 2018-04-12 10:13 ` [PATCH v2 3/4] tpm: Move shared eventlog functions to common.c Thiebaud Weksteen @ 2018-04-20 5:40 ` Jarkko Sakkinen 0 siblings, 0 replies; 21+ messages in thread From: Jarkko Sakkinen @ 2018-04-20 5:40 UTC (permalink / raw) To: Thiebaud Weksteen; +Cc: nayna, linux-integrity, linux-kernel On Thu, Apr 12, 2018 at 12:13:49PM +0200, Thiebaud Weksteen wrote: > Functions and structures specific to TPM1 are renamed from tpm* to tpm1*. > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 4/4] tpm: Move eventlog declarations to its own header 2018-04-12 10:13 [PATCH v2 0/4] Refactor TPM event log code Thiebaud Weksteen ` (2 preceding siblings ...) 2018-04-12 10:13 ` [PATCH v2 3/4] tpm: Move shared eventlog functions to common.c Thiebaud Weksteen @ 2018-04-12 10:13 ` Thiebaud Weksteen 2018-04-20 5:40 ` Jarkko Sakkinen 3 siblings, 1 reply; 21+ messages in thread From: Thiebaud Weksteen @ 2018-04-12 10:13 UTC (permalink / raw) To: jarkko.sakkinen, nayna, linux-integrity, linux-kernel, tweek Cc: linux-integrity, linux-kernel, Thiebaud Weksteen Reduce the size of tpm.h by moving eventlog declarations to a separate header. Signed-off-by: Thiebaud Weksteen <tweek@google.com> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- drivers/char/tpm/eventlog/acpi.c | 1 + drivers/char/tpm/eventlog/common.c | 2 +- drivers/char/tpm/eventlog/common.h | 35 ++++++++++++++++++++++++++++++ drivers/char/tpm/eventlog/efi.c | 1 + drivers/char/tpm/eventlog/of.c | 1 + drivers/char/tpm/eventlog/tpm1.c | 1 + drivers/char/tpm/eventlog/tpm2.c | 1 + drivers/char/tpm/tpm.h | 29 ------------------------- 8 files changed, 41 insertions(+), 30 deletions(-) create mode 100644 drivers/char/tpm/eventlog/common.h diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c index 8476be2e9526..7c53b1973b62 100644 --- a/drivers/char/tpm/eventlog/acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -28,6 +28,7 @@ #include <linux/tpm_eventlog.h> #include "../tpm.h" +#include "common.h" struct acpi_tcpa { struct acpi_table_header hdr; diff --git a/drivers/char/tpm/eventlog/common.c b/drivers/char/tpm/eventlog/common.c index 54934b5a1566..5a8720df2b51 100644 --- a/drivers/char/tpm/eventlog/common.c +++ b/drivers/char/tpm/eventlog/common.c @@ -25,7 +25,7 @@ #include <linux/tpm_eventlog.h> #include "../tpm.h" - +#include "common.h" static int tpm_bios_measurements_open(struct inode *inode, struct file *file) diff --git a/drivers/char/tpm/eventlog/common.h b/drivers/char/tpm/eventlog/common.h new file mode 100644 index 000000000000..47ff8136ceb5 --- /dev/null +++ b/drivers/char/tpm/eventlog/common.h @@ -0,0 +1,35 @@ +#ifndef __TPM_EVENTLOG_COMMON_H__ +#define __TPM_EVENTLOG_COMMON_H__ + +#include "../tpm.h" + +extern const struct seq_operations tpm1_ascii_b_measurements_seqops; +extern const struct seq_operations tpm1_binary_b_measurements_seqops; +extern const struct seq_operations tpm2_binary_b_measurements_seqops; + +#if defined(CONFIG_ACPI) +int tpm_read_log_acpi(struct tpm_chip *chip); +#else +static inline int tpm_read_log_acpi(struct tpm_chip *chip) +{ + return -ENODEV; +} +#endif +#if defined(CONFIG_OF) +int tpm_read_log_of(struct tpm_chip *chip); +#else +static inline int tpm_read_log_of(struct tpm_chip *chip) +{ + return -ENODEV; +} +#endif +#if defined(CONFIG_EFI) +int tpm_read_log_efi(struct tpm_chip *chip); +#else +static inline int tpm_read_log_efi(struct tpm_chip *chip) +{ + return -ENODEV; +} +#endif + +#endif diff --git a/drivers/char/tpm/eventlog/efi.c b/drivers/char/tpm/eventlog/efi.c index e1593c5271a4..68f1e7ee60ce 100644 --- a/drivers/char/tpm/eventlog/efi.c +++ b/drivers/char/tpm/eventlog/efi.c @@ -15,6 +15,7 @@ #include <linux/tpm_eventlog.h> #include "../tpm.h" +#include "common.h" /* read binary bios log from EFI configuration table */ int tpm_read_log_efi(struct tpm_chip *chip) diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c index d98c184d5c13..f8ad4c3ffe8e 100644 --- a/drivers/char/tpm/eventlog/of.c +++ b/drivers/char/tpm/eventlog/of.c @@ -20,6 +20,7 @@ #include <linux/tpm_eventlog.h> #include "../tpm.h" +#include "common.h" int tpm_read_log_of(struct tpm_chip *chip) { diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c index 8f30316e9bb6..58c84784ba25 100644 --- a/drivers/char/tpm/eventlog/tpm1.c +++ b/drivers/char/tpm/eventlog/tpm1.c @@ -29,6 +29,7 @@ #include <linux/tpm_eventlog.h> #include "../tpm.h" +#include "common.h" static const char* tcpa_event_type_strings[] = { diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c index f0723fa9ae14..1b8fa9de2cac 100644 --- a/drivers/char/tpm/eventlog/tpm2.c +++ b/drivers/char/tpm/eventlog/tpm2.c @@ -24,6 +24,7 @@ #include <linux/tpm_eventlog.h> #include "../tpm.h" +#include "common.h" /* * calc_tpm2_event_size() - calculate the event size, where event diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index a583c5001904..067f305e36f0 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -580,35 +580,6 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc, int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc, u8 *buf, size_t *bufsiz); -extern const struct seq_operations tpm1_ascii_b_measurements_seqops; -extern const struct seq_operations tpm1_binary_b_measurements_seqops; -extern const struct seq_operations tpm2_binary_b_measurements_seqops; - -#if defined(CONFIG_ACPI) -int tpm_read_log_acpi(struct tpm_chip *chip); -#else -static inline int tpm_read_log_acpi(struct tpm_chip *chip) -{ - return -ENODEV; -} -#endif -#if defined(CONFIG_OF) -int tpm_read_log_of(struct tpm_chip *chip); -#else -static inline int tpm_read_log_of(struct tpm_chip *chip) -{ - return -ENODEV; -} -#endif -#if defined(CONFIG_EFI) -int tpm_read_log_efi(struct tpm_chip *chip); -#else -static inline int tpm_read_log_efi(struct tpm_chip *chip) -{ - return -ENODEV; -} -#endif - int tpm_bios_log_setup(struct tpm_chip *chip); void tpm_bios_log_teardown(struct tpm_chip *chip); #endif -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/4] tpm: Move eventlog declarations to its own header 2018-04-12 10:13 ` [PATCH v2 4/4] tpm: Move eventlog declarations to its own header Thiebaud Weksteen @ 2018-04-20 5:40 ` Jarkko Sakkinen 0 siblings, 0 replies; 21+ messages in thread From: Jarkko Sakkinen @ 2018-04-20 5:40 UTC (permalink / raw) To: Thiebaud Weksteen; +Cc: nayna, linux-integrity, linux-kernel On Thu, Apr 12, 2018 at 12:13:50PM +0200, Thiebaud Weksteen wrote: > Reduce the size of tpm.h by moving eventlog declarations to a separate > header. > > Signed-off-by: Thiebaud Weksteen <tweek@google.com> > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-05-11 1:51 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-12 10:13 [PATCH v2 0/4] Refactor TPM event log code Thiebaud Weksteen 2018-04-12 10:13 ` [PATCH v2 1/4] tpm: Add explicit endianness cast Thiebaud Weksteen 2018-04-17 3:02 ` Jason Gunthorpe 2018-04-17 8:32 ` Thiebaud Weksteen 2018-04-17 14:00 ` Jason Gunthorpe 2018-04-19 13:09 ` Thiebaud Weksteen 2018-04-20 14:57 ` Jason Gunthorpe 2018-04-23 9:22 ` Thiebaud Weksteen 2018-04-23 10:06 ` Luc Van Oostenryck 2018-04-24 9:57 ` Thiebaud Weksteen 2018-04-20 5:39 ` Jarkko Sakkinen 2018-04-12 10:13 ` [PATCH v2 2/4] tpm: Move eventlog files to a subdirectory Thiebaud Weksteen 2018-04-20 5:39 ` Jarkko Sakkinen 2018-05-07 20:36 ` Mimi Zohar 2018-05-09 14:51 ` Petr Vorel 2018-05-11 1:51 ` Mimi Zohar 2018-05-10 1:39 ` Jarkko Sakkinen 2018-04-12 10:13 ` [PATCH v2 3/4] tpm: Move shared eventlog functions to common.c Thiebaud Weksteen 2018-04-20 5:40 ` Jarkko Sakkinen 2018-04-12 10:13 ` [PATCH v2 4/4] tpm: Move eventlog declarations to its own header Thiebaud Weksteen 2018-04-20 5:40 ` Jarkko Sakkinen
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).