On Wed, 2020-07-15 at 08:21 +0200, Petr Vorel wrote:
> > On Tue, 2020-07-14 at 17:46 +0200, Petr Vorel wrote:
> > > instead of checking in build time as it's runtime dependency.
> > > Also log when tsspcrread not found to make debugging easier.
>
> > > We search for tsspcrread unless there is tss2-esys with Esys_PCR_Read(),
> > > thus pcr_none.c was dropped as unneeded.
>
> > > file_exist(), file_exist() and MIN() taken from LTP project.
>
> > One of these "file_exists" I assume is suppose to be "tst_get_path".
> Yes. I'm sorry, thanks for catching it.
>
>
> > > Signed-off-by: Petr Vorel <pvorel@xxxxxxx>
> > > ---
> > > Hi Mimi,
>
> > > small improvement based on the current next-testing branch
> > > (9638068aff2476b567185d7eb94126449ad89ca7).
>
> > > I'm sorry I don't have the required setup, thus didn't test this patch.
>
> > > Kind regards,
> > > Petr
>
> > Nice! It works.
> Thanks a lot for a testing?
Yes, reviewed/tested together.
diff --git a/src/pcr_tsspcrread.c b/src/pcr_tsspcrread.c
> > > @@ -47,8 +48,21 @@
>
> > > #include "utils.h"
>
> > > -int tpm2_pcr_supported(void)
> > > +#define CMD "tsspcrread"
> > > +
> > > +static char path[PATH_MAX];
> > > +
> > > +int tpm2_pcr_supported(char **errmsg)
> > > {
> > > + int ret;
> > > +
> > > + if (get_cmd_path(CMD, path, sizeof(path))) {
> > > + ret = asprintf(errmsg, "Couldn't find '%s' in $PATH", CMD);
> > > + if (ret == -1) /* the contents of errmsg is undefined */
> > > + *errmsg = NULL;
> > > + return 0;
> > > + }
> > > +
>
> > Any chance you could also emit the pathname on success as well?
>
> Do you mean to print it into stderr:
>
> int tpm2_pcr_supported(char **errmsg)
> {
> int ret;
>
> if (get_cmd_path(CMD, path, sizeof(path))) {
> ret = asprintf(errmsg, "Couldn't find '%s' in $PATH", CMD);
> if (ret == -1) /* the contents of errmsg is undefined */
> *errmsg = NULL;
> return 0;
> }
>
> ret = asprintf(errmsg, "Found '%s' in $PATH", CMD);
> if (ret == -1) /* the contents of errmsg is undefined */
> *errmsg = NULL;
> return 1;
> }
>
When running these tests remotely, it helps to know which method of
reading the PCRs is used. How about adding something like this to
both instances of tpm2_pcr_supported()?
if (imaevm_params.verbose > LOG_INFO)
log_info("Using %s to read PCRs.\n", CMD);
> Shell I post v2 or you amend my patch?
Either way is fine.
> BTW I was thinking to create custom function / macro for handling errmsg to
> reduce duplicity.
Sure, I assume that would be in addition to log_err() and log_errno().
>
> + there is minor warning on newer gcc, I'm not sure how to fix that:
>
> evmctl.c: In function ‘read_tpm_banks’:
> evmctl.c:1404:25: warning: ‘%2.2d’ directive writing between 2 and 10 bytes into a region of size 3 [-Wformat-overflow=]
> 1404 | sprintf(pcr_str, "PCR-%2.2d", i);
> | ^~~~~
> evmctl.c:1404:20: note: directive argument in the range [0, 2147483647]
> 1404 | sprintf(pcr_str, "PCR-%2.2d", i);
> | ^~~~~~~~~~~
> evmctl.c:1404:3: note: ‘sprintf’ output between 7 and 15 bytes into a destination of size 7
> 1404 | sprintf(pcr_str, "PCR-%2.2d", i);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Interesting. Checking that "i" isn't greater than 99 solves this
warning. Changing pcr_str size from 7 to 8 solves the other warning.
Mimi
