Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test eventlog: fix check eventlog.sh if efivar.h exists #3304

Merged

Conversation

JuergenReppSIT
Copy link
Member

If efivar.h exist a pretty print function for the DevicePath is executed. Therefore two yaml test files are needed for the bin test file uefiservices.
Fixes #3302.
Signed-off-by: Juergen Repp [email protected]

@JuergenReppSIT JuergenReppSIT force-pushed the fix-eventlog-pretty-print branch from a08f85e to c3a814a Compare November 20, 2023 14:12
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e988a22) 76.53% compared to head (3bd92de) 76.53%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3304   +/-   ##
=======================================
  Coverage   76.53%   76.53%           
=======================================
  Files         173      173           
  Lines       23663    23663           
=======================================
  Hits        18110    18110           
  Misses       5553     5553           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JuergenReppSIT JuergenReppSIT changed the title test eventlog: fix check evntlog.sh if efivar.h exists test eventlog: fix check eventlog.sh if efivar.h exists Nov 20, 2023
@JuergenReppSIT JuergenReppSIT force-pushed the fix-eventlog-pretty-print branch 2 times, most recently from 62b9b93 to c570534 Compare November 20, 2023 14:47
Copy link
Member

@AndreasFuchsTPM AndreasFuchsTPM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should instead make sure that the outputs are always the same, no matter if evivar.h is here or not.

@AndreasFuchsTPM
Copy link
Member

So if I understand this correctly:

  • If efivar.h is present on the system, the tool outputs a pretty print.
  • If efivar.h is not present on the system, the tool outputs a raw format print.
    This is not acceptable, we need to decide on either pretty print or raw print or make this a CML-line option (e.g. --pretty is only available if efivar.h is present).

@JuergenReppSIT @williamcroberts Thoughts ?

@JuergenReppSIT
Copy link
Member Author

  This is not acceptable, we need to decide on either pretty print or raw print or make this a CML-line option (e.g. --pretty is only available if efivar.h is present).

@JuergenReppSIT @williamcroberts Thoughts ?

@AndreasFuchsTPM @williamcroberts none of the alternatives is really good. I would prefer the one with the CML option --pretty

@williamcroberts williamcroberts added this to the next milestone Dec 20, 2023
@williamcroberts
Copy link
Member

@JuergenReppSIT or @AndreasFuchsTPM looks like FreeBSD in Cirrus is unhappy with:

0000: 00000000                          ....
This test failed due to a TPM bug regarding signed comparison as described
in TCG's Errata for TCG Trusted Platform Module Library Revision 1.59 Version 1.4,
Section 2.5 TPM_EO – two’s complement

Have you seen this before? If so what's the fix? update the tpm simulator binary?

If efivar.h exist a pretty print function for the DevicePath
is executed. Therefore two yaml test files are needed for
the bin test file uefiservices.
Fixes tpm2-software#3302.

Signed-off-by: Juergen Repp <[email protected]>
@JuergenReppSIT JuergenReppSIT force-pushed the fix-eventlog-pretty-print branch from c570534 to 3bd92de Compare December 20, 2023 21:06
@JuergenReppSIT
Copy link
Member Author

Have you seen this before? If so what's the fix? update the tpm simulator binary?

@williamcroberts This problem was fixed with #3322. I didn't rebase the PR because I wanted to hear your opinion to the pretty print problem first.

@williamcroberts
Copy link
Member

Have you seen this before? If so what's the fix? update the tpm simulator binary?

@williamcroberts This problem was fixed with #3322. I didn't rebase the PR because I wanted to hear your opinion to the pretty print problem first.

We just leave the behavior as is. While not desirable, we will fix for 6.0.0. I removed breaking change, because this isn't a breaking change.

@williamcroberts williamcroberts merged commit 196e3d4 into tpm2-software:master Dec 20, 2023
18 of 19 checks passed
@JuergenReppSIT JuergenReppSIT deleted the fix-eventlog-pretty-print branch February 20, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing test: test/integration/tests/eventlog.sh
3 participants