From f9568765d4d3d57de1ec01d85f0a0682920f4d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 5 Dec 2023 19:02:14 +0100 Subject: [PATCH 1/3] variuos: fwrite() does not set errno The man page doesn't even mention errno. It just says that ferror() should be used to check for errors. Those writes are unlikely to fail, but if they do, errno might even be 0. Also, we have fflush_and_check() which does additional paranoia around errno, because we apparently do not trust that errno will always be set correctly. --- src/analyze/analyze-srk.c | 10 +++++++--- src/creds/creds.c | 7 ++++--- src/libsystemd/sd-journal/journal-verify.c | 2 +- src/tpm2-setup/tpm2-setup.c | 6 ++++-- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/analyze/analyze-srk.c b/src/analyze/analyze-srk.c index 31382462254..0e24b416bb4 100644 --- a/src/analyze/analyze-srk.c +++ b/src/analyze/analyze-srk.c @@ -2,6 +2,7 @@ #include "analyze.h" #include "analyze-srk.h" +#include "fileio.h" #include "tpm2-util.h" int verb_srk(int argc, char *argv[], void *userdata) { @@ -33,12 +34,15 @@ int verb_srk(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to marshal SRK: %m"); if (isatty(STDOUT_FILENO)) - return log_error_errno(SYNTHETIC_ERRNO(EIO), "Refusing to write binary data to TTY, please redirect output to file."); + return log_error_errno(SYNTHETIC_ERRNO(EIO), + "Refusing to write binary data to TTY, please redirect output to file."); if (fwrite(marshalled, 1, marshalled_size, stdout) != marshalled_size) - return log_error_errno(errno, "Failed to write SRK to stdout: %m"); + return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to write SRK to stdout: %m"); - fflush(stdout); + r = fflush_and_check(stdout); + if (r < 0) + return log_error_errno(r, "Failed to write SRK to stdout: %m"); return EXIT_SUCCESS; #else diff --git a/src/creds/creds.c b/src/creds/creds.c index 101e5abf9be..7a98a5dcd34 100644 --- a/src/creds/creds.c +++ b/src/creds/creds.c @@ -350,14 +350,15 @@ static int write_blob(FILE *f, const void *data, size_t size) { } if (fwrite(data, 1, size, f) != size) - return log_error_errno(errno, "Failed to write credential data: %m"); + return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to write credential data: %m"); r = print_newline(f, data, size); if (r < 0) return r; - if (fflush(f) != 0) - return log_error_errno(errno, "Failed to flush output: %m"); + r = fflush_and_check(f); + if (r < 0) + return log_error_errno(r, "Failed to flush output: %m"); return 0; } diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 8fc53beb42e..bdaa01d66fa 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -384,7 +384,7 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o static int write_uint64(FILE *fp, uint64_t p) { if (fwrite(&p, sizeof(p), 1, fp) != 1) - return -errno; + return -EIO; return 0; } diff --git a/src/tpm2-setup/tpm2-setup.c b/src/tpm2-setup/tpm2-setup.c index be34d166d7a..0be7ffc6a5f 100644 --- a/src/tpm2-setup/tpm2-setup.c +++ b/src/tpm2-setup/tpm2-setup.c @@ -284,7 +284,8 @@ static int run(int argc, char *argv[]) { if (runtime_key.pkey) { if (memcmp_nn(tpm2_key.fingerprint, tpm2_key.fingerprint_size, runtime_key.fingerprint, runtime_key.fingerprint_size) != 0) - return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), "Saved runtime SRK differs from TPM SRK, refusing."); + return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), + "Saved runtime SRK differs from TPM SRK, refusing."); if (arg_early) { log_info("SRK saved in '%s' matches SRK in TPM2.", runtime_key.path); @@ -351,7 +352,8 @@ static int run(int argc, char *argv[]) { return log_error_errno(r, "Failed to marshal TPM2_PUBLIC key."); if (fwrite(marshalled, 1, marshalled_size, f) != marshalled_size) - return log_error_errno(errno, "Failed to write SRK public key file '%s'.", tpm2b_public_path); + return log_error_errno(SYNTHETIC_ERRNO(EIO), + "Failed to write SRK public key file '%s'.", tpm2b_public_path); if (fchmod(fileno(f), 0444) < 0) return log_error_errno(errno, "Failed to adjust access mode of SRK public key file '%s' to 0444: %m", tpm2b_public_path); From d333b236f4da9b791a3e8d639cb2ff44df3d805c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 5 Dec 2023 19:03:26 +0100 Subject: [PATCH 2/3] networkd: unvoidify fwrite() Those were the only two places we did that, so for consistency it's better to drop it. --- src/network/networkd-lldp-rx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-lldp-rx.c b/src/network/networkd-lldp-rx.c index 3a5988405f2..c45d3e32d74 100644 --- a/src/network/networkd-lldp-rx.c +++ b/src/network/networkd-lldp-rx.c @@ -147,8 +147,8 @@ int link_lldp_save(Link *link) { goto finish; u = htole64(sz); - (void) fwrite(&u, 1, sizeof(u), f); - (void) fwrite(p, 1, sz, f); + fwrite(&u, 1, sizeof(u), f); + fwrite(p, 1, sz, f); } r = fflush_and_check(f); From 26e82eef016abad62cfa6c8d80c4e561e5f4b67d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 5 Dec 2023 19:17:20 +0100 Subject: [PATCH 3/3] sd-journal/catalog: modernize write_catalog() --- src/libsystemd/sd-journal/catalog.c | 50 +++++++++-------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index ae91534198c..4c5562ba488 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -16,6 +16,7 @@ #include "conf-files.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "hashmap.h" #include "log.h" #include "memory-util.h" @@ -382,10 +383,8 @@ static int64_t write_catalog( CatalogItem *items, size_t n) { + _cleanup_(unlink_and_freep) char *p = NULL; _cleanup_fclose_ FILE *w = NULL; - _cleanup_free_ char *p = NULL; - CatalogHeader header; - size_t k; int r; r = mkdir_parents(database, 0755); @@ -394,54 +393,35 @@ static int64_t write_catalog( r = fopen_temporary(database, &w, &p); if (r < 0) - return log_error_errno(r, "Failed to open database for writing: %s: %m", - database); + return log_error_errno(r, "Failed to open database for writing: %s: %m", database); - header = (CatalogHeader) { + CatalogHeader header = { .signature = CATALOG_SIGNATURE, .header_size = htole64(CONST_ALIGN_TO(sizeof(CatalogHeader), 8)), .catalog_item_size = htole64(sizeof(CatalogItem)), .n_items = htole64(n), }; - r = -EIO; + if (fwrite(&header, sizeof(header), 1, w) != 1) + return log_error_errno(SYNTHETIC_ERRNO(EIO), "%s: failed to write header.", p); - k = fwrite(&header, 1, sizeof(header), w); - if (k != sizeof(header)) { - log_error("%s: failed to write header.", p); - goto error; - } + if (fwrite(items, sizeof(CatalogItem), n, w) != n) + return log_error_errno(SYNTHETIC_ERRNO(EIO), "%s: failed to write database.", p); - k = fwrite(items, 1, n * sizeof(CatalogItem), w); - if (k != n * sizeof(CatalogItem)) { - log_error("%s: failed to write database.", p); - goto error; - } - - k = fwrite(sb->buf, 1, sb->len, w); - if (k != sb->len) { - log_error("%s: failed to write strings.", p); - goto error; - } + if (fwrite(sb->buf, sb->len, 1, w) != 1) + return log_error_errno(SYNTHETIC_ERRNO(EIO), "%s: failed to write strings.", p); r = fflush_and_check(w); - if (r < 0) { - log_error_errno(r, "%s: failed to write database: %m", p); - goto error; - } + if (r < 0) + return log_error_errno(r, "%s: failed to write database: %m", p); (void) fchmod(fileno(w), 0644); - if (rename(p, database) < 0) { - r = log_error_errno(errno, "rename (%s -> %s) failed: %m", p, database); - goto error; - } + if (rename(p, database) < 0) + return log_error_errno(errno, "rename (%s -> %s) failed: %m", p, database); + p = mfree(p); /* free without unlinking */ return ftello(w); - -error: - (void) unlink(p); - return r; } int catalog_update(const char* database, const char* root, const char* const* dirs) {