Mimi,
On Thu, Nov 03, 2022 at 02:38:49PM -0400, Mimi Zohar wrote:
> Define a log_errno_reset macro to emit the errno string at or near the
> time of error, similar to the existing log_errno macro, but also reset
> errno to avoid dangling or duplicate errno messages on exit.
>
> The initial usage is for non-critical file open failures.
>
> Suggested-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> ---
> src/evmctl.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
Reviewed-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 0412bc0ac2b0..54123bf20f03 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -166,6 +166,9 @@ struct tpm_bank_info {
> static char *pcrfile[MAX_PCRFILE];
> static unsigned npcrfile;
>
> +#define log_errno_reset(level, fmt, args...) \
> + {do_log(level, fmt " (errno: %s)\n", ##args, strerror(errno)); errno = 0; }
> +
> static int bin2file(const char *file, const char *ext, const unsigned char *data, int len)
> {
> FILE *fp;
> @@ -1911,8 +1914,10 @@ static int read_sysfs_pcrs(int num_banks, struct tpm_bank_info *tpm_banks)
> fp = fopen(pcrs, "r");
> if (!fp)
> fp = fopen(misc_pcrs, "r");
> - if (!fp)
> + if (!fp) {
> + log_errno_reset(LOG_DEBUG, "Failed to read TPM 1.2 PCRs");
> return -1;
> + }
>
> result = read_one_bank(&tpm_banks[0], fp);
> fclose(fp);
> @@ -2055,7 +2060,6 @@ static int ima_measurement(const char *file)
> int err_padded = -1;
> int err = -1;
>
> - errno = 0;
> memset(zero, 0, MAX_DIGEST_SIZE);
>
> pseudo_padded_banks = init_tpm_banks(&num_banks);
> @@ -2072,6 +2076,8 @@ static int ima_measurement(const char *file)
> init_public_keys(imaevm_params.keyfile);
> else /* assume read pubkey from x509 cert */
> init_public_keys("/etc/keys/x509_evm.der");
> + if (errno)
> + log_errno_reset(LOG_DEBUG, "Failed to initialize public keys");
Library prints appropriate error messages, so this is perhaps just to
clear errno. But it's not necessarily completely failed, but maybe
failure in one key. So I would say "Failure in initializing public
keys" to be precise.
ps.
BTW, init_public_keys API call cannot return error except by errno,
but it does not set it consistently so some errors may be missed.
init_public_keys loops calling read_pub_pkey
entry->key = read_pub_pkey(keyfile, 1);
if (!entry->key) {
free(entry);
continue;
}
and read_pub_pkey have such code:
if (!keyfile)
return NULL;
In that case some key is not read but we don't get any error notification.
I think it's legal, by the right of being library, so set `errno =
EINVAL` there somewhere. But, I'm not sure where - as we should not
clobber existing errno values. Perhaps, errno setting should be added to
libimaevm consistently to all functions, but this is huge task, so I
would not suggest to do it now. Just suggestion for the future
developments, maybe.
Thanks,
>
> /*
> * Reading the PCRs before walking the IMA measurement list
> @@ -2746,6 +2752,8 @@ int main(int argc, char *argv[])
> unsigned long keyid;
> char *eptr;
>
> + errno = 0; /* initialize global errno */
> +
> #if !(OPENSSL_VERSION_NUMBER < 0x10100000)
> OPENSSL_init_crypto(
> #ifndef DISABLE_OPENSSL_CONF
> --
> 2.31.1
