| Message ID | 20221101201803.372652-3-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | address deprecated warnings | expand |
On 11/1/22 16:17, 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. After looking just at the fopen() in evmctl.c at the end of this series there are some that are left over that show no error message (read_binary_bios_measurements) others that still use log_err() then. Should they not all be converted/extended and use log_errno_reset()? > > Suggested-by: Vitaly Chikunov <vt@altlinux.org> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > src/evmctl.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > 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"); > > /* > * 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
On Wed, 2022-11-02 at 17:02 -0400, Stefan Berger wrote: > > On 11/1/22 16:17, 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. > > After looking just at the fopen() in evmctl.c at the end of this > series there are some that are left over that show no error message > (read_binary_bios_measurements) others that still use log_err() then. > Should they not all be converted/extended and use log_errno_reset()? > No, log_errno_reset() is meant for the specific case where the program continues to execute, but the errno message is delayed and emitted on program exit. In the case of read_binary_bios_measurements(), the caller emits an error message and immediately exits. There's no delay between the error and the errno message. Calling log_err() to emit an error mesage and then exiting seems to be fine.
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"); /* * 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
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@altlinux.org> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- src/evmctl.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)