From 0ce8870f19a839c7b09b4ef5b61c2d363050c7d9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 14:13:47 +0900 Subject: [PATCH 1/9] string-util: introduce ascii_ishex() --- src/fundamental/string-util-fundamental.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fundamental/string-util-fundamental.h b/src/fundamental/string-util-fundamental.h index ecf32e519f..d823147881 100644 --- a/src/fundamental/string-util-fundamental.h +++ b/src/fundamental/string-util-fundamental.h @@ -110,6 +110,10 @@ static inline bool ascii_isdigit(sd_char a) { return a >= '0' && a <= '9'; } +static inline bool ascii_ishex(sd_char a) { + return ascii_isdigit(a) || (a >= 'a' && a <= 'f') || (a >= 'A' && a <= 'F'); +} + static inline bool ascii_isalpha(sd_char a) { /* A pure ASCII, locale independent version of isalpha() */ return (a >= 'a' && a <= 'z') || (a >= 'A' && a <= 'Z'); From 28bf2de201e890193b57accbf736c7d3d82d813a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 14:20:03 +0900 Subject: [PATCH 2/9] sd-id128: several cleanups - use SD_ID128_STRING_MAX or friend, - use sizeof(sd_id128_t), - use newly introduced ascii_ishex(). --- src/libsystemd/sd-id128/id128-util.c | 58 ++++++++++------------------ src/libsystemd/sd-id128/sd-id128.c | 21 +++++----- 2 files changed, 33 insertions(+), 46 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 4f52c14f64..1085486ae2 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -13,50 +13,35 @@ #include "sync-util.h" bool id128_is_valid(const char *s) { - size_t i, l; + size_t l; assert(s); l = strlen(s); - if (l == 32) { + if (l == SD_ID128_STRING_MAX - 1) /* Plain formatted 128bit hex string */ + return in_charset(s, HEXDIGITS); - for (i = 0; i < l; i++) { - char c = s[i]; - - if (!ascii_isdigit(c) && - !(c >= 'a' && c <= 'f') && - !(c >= 'A' && c <= 'F')) - return false; - } - - } else if (l == 36) { - + if (l == SD_ID128_UUID_STRING_MAX - 1) { /* Formatted UUID */ - - for (i = 0; i < l; i++) { + for (size_t i = 0; i < l; i++) { char c = s[i]; if (IN_SET(i, 8, 13, 18, 23)) { if (c != '-') return false; - } else { - if (!ascii_isdigit(c) && - !(c >= 'a' && c <= 'f') && - !(c >= 'A' && c <= 'F')) - return false; - } + } else if (!ascii_ishex(c)) + return false; } + return true; + } - } else - return false; - - return true; + return false; } int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { - char buffer[36 + 2]; + char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ ssize_t l; assert(fd >= 0); @@ -80,28 +65,28 @@ int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { /* Treat an "uninitialized" id file like an empty one */ return f == ID128_PLAIN_OR_UNINIT && strneq(buffer, "uninitialized\n", l) ? -ENOMEDIUM : -EINVAL; - case 33: /* plain UUID with trailing newline */ - if (buffer[32] != '\n') + case SD_ID128_STRING_MAX: /* plain UUID with trailing newline */ + if (buffer[SD_ID128_STRING_MAX-1] != '\n') return -EINVAL; _fallthrough_; - case 32: /* plain UUID without trailing newline */ + case SD_ID128_STRING_MAX-1: /* plain UUID without trailing newline */ if (f == ID128_UUID) return -EINVAL; - buffer[32] = 0; + buffer[SD_ID128_STRING_MAX-1] = 0; break; - case 37: /* RFC UUID with trailing newline */ - if (buffer[36] != '\n') + case SD_ID128_UUID_STRING_MAX: /* RFC UUID with trailing newline */ + if (buffer[SD_ID128_UUID_STRING_MAX-1] != '\n') return -EINVAL; _fallthrough_; - case 36: /* RFC UUID without trailing newline */ + case SD_ID128_UUID_STRING_MAX-1: /* RFC UUID without trailing newline */ if (IN_SET(f, ID128_PLAIN, ID128_PLAIN_OR_UNINIT)) return -EINVAL; - buffer[36] = 0; + buffer[SD_ID128_UUID_STRING_MAX-1] = 0; break; default: @@ -122,7 +107,7 @@ int id128_read(const char *p, Id128Format f, sd_id128_t *ret) { } int id128_write_fd(int fd, Id128Format f, sd_id128_t id, bool do_sync) { - char buffer[36 + 2]; + char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ size_t sz; int r; @@ -131,14 +116,13 @@ int id128_write_fd(int fd, Id128Format f, sd_id128_t id, bool do_sync) { if (f != ID128_UUID) { assert_se(sd_id128_to_string(id, buffer)); - buffer[SD_ID128_STRING_MAX - 1] = '\n'; sz = SD_ID128_STRING_MAX; } else { assert_se(sd_id128_to_uuid_string(id, buffer)); - buffer[SD_ID128_UUID_STRING_MAX - 1] = '\n'; sz = SD_ID128_UUID_STRING_MAX; } + buffer[sz - 1] = '\n'; r = loop_write(fd, buffer, sz, false); if (r < 0) return r; diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index c8f051196d..922b94c5ed 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -18,14 +18,17 @@ #include "user-util.h" _public_ char *sd_id128_to_string(sd_id128_t id, char s[_SD_ARRAY_STATIC SD_ID128_STRING_MAX]) { + size_t k = 0; + assert_return(s, NULL); - for (size_t n = 0; n < 16; n++) { - s[n*2] = hexchar(id.bytes[n] >> 4); - s[n*2+1] = hexchar(id.bytes[n] & 0xF); + for (size_t n = 0; n < sizeof(sd_id128_t); n++) { + s[k++] = hexchar(id.bytes[n] >> 4); + s[k++] = hexchar(id.bytes[n] & 0xF); } - s[SD_ID128_STRING_MAX-1] = 0; + assert(k == SD_ID128_STRING_MAX - 1); + s[k] = 0; return s; } @@ -37,7 +40,7 @@ _public_ char *sd_id128_to_uuid_string(sd_id128_t id, char s[_SD_ARRAY_STATIC SD /* Similar to sd_id128_to_string() but formats the result as UUID instead of plain hex chars */ - for (size_t n = 0; n < 16; n++) { + for (size_t n = 0; n < sizeof(sd_id128_t); n++) { if (IN_SET(n, 4, 6, 8, 10)) s[k++] = '-'; @@ -52,14 +55,14 @@ _public_ char *sd_id128_to_uuid_string(sd_id128_t id, char s[_SD_ARRAY_STATIC SD return s; } -_public_ int sd_id128_from_string(const char s[], sd_id128_t *ret) { - unsigned n, i; +_public_ int sd_id128_from_string(const char *s, sd_id128_t *ret) { + size_t n, i; sd_id128_t t; bool is_guid = false; assert_return(s, -EINVAL); - for (n = 0, i = 0; n < 16;) { + for (n = 0, i = 0; n < sizeof(sd_id128_t);) { int a, b; if (s[i] == '-') { @@ -89,7 +92,7 @@ _public_ int sd_id128_from_string(const char s[], sd_id128_t *ret) { t.bytes[n++] = (a << 4) | b; } - if (i != (is_guid ? 36 : 32)) + if (i != (is_guid ? SD_ID128_UUID_STRING_MAX : SD_ID128_STRING_MAX) - 1) return -EINVAL; if (s[i] != 0) From 057bf780e9d45480fbacdd3b060dbe37b37f9693 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 14:30:31 +0900 Subject: [PATCH 3/9] sd-id128: make id128_read() or friends return -ENOPKG when the file contents is "uninitialized" Then, this drops ID128_PLAIN_OR_UNINIT. Also, this renames Id128Format -> Id128FormatFlag, and make it bitfield. Fixes #25634. --- src/boot/bootctl.c | 2 +- src/libsystemd/sd-id128/id128-util.c | 35 +++++++++++--------- src/libsystemd/sd-id128/id128-util.h | 23 +++++-------- src/libsystemd/sd-id128/sd-id128.c | 4 +-- src/libsystemd/sd-journal/journal-file.c | 2 +- src/machine-id-setup/machine-id-setup-main.c | 2 +- src/nspawn/nspawn.c | 6 ++-- src/partition/repart.c | 4 +-- src/shared/discover-image.c | 2 +- src/shared/machine-id-setup.c | 12 +++---- src/shared/specifier.c | 2 +- src/test/test-condition.c | 2 +- src/test/test-fs-util.c | 2 +- src/test/test-id128.c | 26 +++++++-------- 14 files changed, 61 insertions(+), 63 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 0df456827c..44a5a371b9 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -179,7 +179,7 @@ static int load_etc_machine_id(void) { int r; r = sd_id128_get_machine(&arg_machine_id); - if (IN_SET(r, -ENOENT, -ENOMEDIUM)) /* Not set or empty */ + if (IN_SET(r, -ENOENT, -ENOMEDIUM, -ENOPKG)) /* Not set or empty */ return 0; if (r < 0) return log_error_errno(r, "Failed to get machine-id: %m"); diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 1085486ae2..2cf8848981 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -40,17 +40,21 @@ bool id128_is_valid(const char *s) { return false; } -int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { +int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret) { char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ ssize_t l; assert(fd >= 0); - assert(f < _ID128_FORMAT_MAX); /* Reads an 128bit ID from a file, which may either be in plain format (32 hex digits), or in UUID format, both * optionally followed by a newline and nothing else. ID files should really be newline terminated, but if they * aren't that's OK too, following the rule of "Be conservative in what you send, be liberal in what you - * accept". */ + * accept". + * + * This returns the following: + * -ENOMEDIUM: an empty string, + * -ENOPKG: "uninitialized" or "uninitialized\n", + * -EINVAL: other invalid strings. */ l = loop_read(fd, buffer, sizeof(buffer), false); /* we expect a short read of either 32/33 or 36/37 chars */ if (l < 0) @@ -60,10 +64,9 @@ int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { switch (l) { - case 13: - case 14: - /* Treat an "uninitialized" id file like an empty one */ - return f == ID128_PLAIN_OR_UNINIT && strneq(buffer, "uninitialized\n", l) ? -ENOMEDIUM : -EINVAL; + case STRLEN("uninitialized"): + case STRLEN("uninitialized\n"): + return strneq(buffer, "uninitialized\n", l) ? -ENOPKG : -EINVAL; case SD_ID128_STRING_MAX: /* plain UUID with trailing newline */ if (buffer[SD_ID128_STRING_MAX-1] != '\n') @@ -71,7 +74,7 @@ int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { _fallthrough_; case SD_ID128_STRING_MAX-1: /* plain UUID without trailing newline */ - if (f == ID128_UUID) + if (!FLAGS_SET(f, ID128_FORMAT_PLAIN)) return -EINVAL; buffer[SD_ID128_STRING_MAX-1] = 0; @@ -83,7 +86,7 @@ int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { _fallthrough_; case SD_ID128_UUID_STRING_MAX-1: /* RFC UUID without trailing newline */ - if (IN_SET(f, ID128_PLAIN, ID128_PLAIN_OR_UNINIT)) + if (!FLAGS_SET(f, ID128_FORMAT_UUID)) return -EINVAL; buffer[SD_ID128_UUID_STRING_MAX-1] = 0; @@ -96,7 +99,7 @@ int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret) { return sd_id128_from_string(buffer, ret); } -int id128_read(const char *p, Id128Format f, sd_id128_t *ret) { +int id128_read(const char *p, Id128FormatFlag f, sd_id128_t *ret) { _cleanup_close_ int fd = -1; fd = open(p, O_RDONLY|O_CLOEXEC|O_NOCTTY); @@ -106,15 +109,15 @@ int id128_read(const char *p, Id128Format f, sd_id128_t *ret) { return id128_read_fd(fd, f, ret); } -int id128_write_fd(int fd, Id128Format f, sd_id128_t id, bool do_sync) { +int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id, bool do_sync) { char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ size_t sz; int r; assert(fd >= 0); - assert(f < _ID128_FORMAT_MAX); + assert(IN_SET((f & ID128_FORMAT_ANY), ID128_FORMAT_PLAIN, ID128_FORMAT_UUID)); - if (f != ID128_UUID) { + if (FLAGS_SET(f, ID128_FORMAT_PLAIN)) { assert_se(sd_id128_to_string(id, buffer)); sz = SD_ID128_STRING_MAX; } else { @@ -136,7 +139,7 @@ int id128_write_fd(int fd, Id128Format f, sd_id128_t id, bool do_sync) { return 0; } -int id128_write(const char *p, Id128Format f, sd_id128_t id, bool do_sync) { +int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id, bool do_sync) { _cleanup_close_ int fd = -1; fd = open(p, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444); @@ -178,9 +181,9 @@ int id128_get_product(sd_id128_t *ret) { /* Reads the systems product UUID from DMI or devicetree (where it is located on POWER). This is * particularly relevant in VM environments, where VM managers typically place a VM uuid there. */ - r = id128_read("/sys/class/dmi/id/product_uuid", ID128_UUID, &uuid); + r = id128_read("/sys/class/dmi/id/product_uuid", ID128_FORMAT_UUID, &uuid); if (r == -ENOENT) - r = id128_read("/proc/device-tree/vm,uuid", ID128_UUID, &uuid); + r = id128_read("/proc/device-tree/vm,uuid", ID128_FORMAT_UUID, &uuid); if (r < 0) return r; diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index 17b180c10c..bb237cb6cc 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -10,22 +10,17 @@ bool id128_is_valid(const char *s) _pure_; -typedef enum Id128Format { - ID128_ANY, - ID128_PLAIN, /* formatted as 32 hex chars as-is */ - ID128_PLAIN_OR_UNINIT, /* formatted as 32 hex chars as-is; allow special "uninitialized" - * value when reading from file (id128_read() and id128_read_fd()). - * - * This format should be used when reading a machine-id file. */ - ID128_UUID, /* formatted as 36 character uuid string */ - _ID128_FORMAT_MAX, -} Id128Format; +typedef enum Id128FormatFlag { + ID128_FORMAT_PLAIN = 1 << 0, /* formatted as 32 hex chars as-is */ + ID128_FORMAT_UUID = 1 << 1, /* formatted as 36 character uuid string */ + ID128_FORMAT_ANY = ID128_FORMAT_PLAIN | ID128_FORMAT_UUID, +} Id128FormatFlag; -int id128_read_fd(int fd, Id128Format f, sd_id128_t *ret); -int id128_read(const char *p, Id128Format f, sd_id128_t *ret); +int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret); +int id128_read(const char *p, Id128FormatFlag f, sd_id128_t *ret); -int id128_write_fd(int fd, Id128Format f, sd_id128_t id, bool do_sync); -int id128_write(const char *p, Id128Format f, sd_id128_t id, bool do_sync); +int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id, bool do_sync); +int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id, bool do_sync); void id128_hash_func(const sd_id128_t *p, struct siphash *state); int id128_compare_func(const sd_id128_t *a, const sd_id128_t *b) _pure_; diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index 922b94c5ed..a5758e5841 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -126,7 +126,7 @@ _public_ int sd_id128_get_machine(sd_id128_t *ret) { assert_return(ret, -EINVAL); if (sd_id128_is_null(saved_machine_id)) { - r = id128_read("/etc/machine-id", ID128_PLAIN, &saved_machine_id); + r = id128_read("/etc/machine-id", ID128_FORMAT_PLAIN, &saved_machine_id); if (r < 0) return r; @@ -145,7 +145,7 @@ _public_ int sd_id128_get_boot(sd_id128_t *ret) { assert_return(ret, -EINVAL); if (sd_id128_is_null(saved_boot_id)) { - r = id128_read("/proc/sys/kernel/random/boot_id", ID128_UUID, &saved_boot_id); + r = id128_read("/proc/sys/kernel/random/boot_id", ID128_FORMAT_UUID, &saved_boot_id); if (r < 0) return r; } diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 507958dabd..ab518fea80 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -384,7 +384,7 @@ static int journal_file_refresh_header(JournalFile *f) { assert(f->header); r = sd_id128_get_machine(&f->header->machine_id); - if (IN_SET(r, -ENOENT, -ENOMEDIUM)) + if (IN_SET(r, -ENOENT, -ENOMEDIUM, -ENOPKG)) /* We don't have a machine-id, let's continue without */ zero(f->header->machine_id); else if (r < 0) diff --git a/src/machine-id-setup/machine-id-setup-main.c b/src/machine-id-setup/machine-id-setup-main.c index 6aef48650d..9101f4e11a 100644 --- a/src/machine-id-setup/machine-id-setup-main.c +++ b/src/machine-id-setup/machine-id-setup-main.c @@ -164,7 +164,7 @@ static int run(int argc, char *argv[]) { return r; etc_machine_id = prefix_roota(arg_root, "/etc/machine-id"); - r = id128_read(etc_machine_id, ID128_PLAIN, &id); + r = id128_read(etc_machine_id, ID128_FORMAT_PLAIN, &id); if (r < 0) return log_error_errno(r, "Failed to read machine ID back: %m"); } else { diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 1282c8b98b..0618ab2313 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2196,7 +2196,7 @@ static int setup_boot_id(void) { if (r < 0) return log_error_errno(r, "Failed to generate random boot id: %m"); - r = id128_write(path, ID128_UUID, rnd, false); + r = id128_write(path, ID128_FORMAT_UUID, rnd, false); if (r < 0) return log_error_errno(r, "Failed to write boot id: %m"); @@ -2830,9 +2830,9 @@ static int setup_machine_id(const char *directory) { etc_machine_id = prefix_roota(directory, "/etc/machine-id"); - r = id128_read(etc_machine_id, ID128_PLAIN_OR_UNINIT, &id); + r = id128_read(etc_machine_id, ID128_FORMAT_PLAIN, &id); if (r < 0) { - if (!IN_SET(r, -ENOENT, -ENOMEDIUM)) /* If the file is missing or empty, we don't mind */ + if (!IN_SET(r, -ENOENT, -ENOMEDIUM, -ENOPKG)) /* If the file is missing, empty, or uninitialized, we don't mind */ return log_error_errno(r, "Failed to read machine ID from container image: %m"); if (sd_id128_is_null(arg_uuid)) { diff --git a/src/partition/repart.c b/src/partition/repart.c index a627d84305..1e647e2ec9 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -4741,8 +4741,8 @@ static int context_read_seed(Context *context, const char *root) { else if (fd < 0) return log_error_errno(fd, "Failed to determine machine ID of image: %m"); else { - r = id128_read_fd(fd, ID128_PLAIN_OR_UNINIT, &context->seed); - if (r == -ENOMEDIUM) + r = id128_read_fd(fd, ID128_FORMAT_PLAIN, &context->seed); + if (IN_SET(r, -ENOMEDIUM, -ENOPKG)) log_info("No machine ID set, using randomized partition UUIDs."); else if (r < 0) return log_error_errno(r, "Failed to parse machine ID of image: %m"); diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 8bc165ef4c..eb4d23f0e8 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -1158,7 +1158,7 @@ int image_read_metadata(Image *i) { if (fd < 0) log_debug_errno(errno, "Failed to open %s: %m", path); else { - r = id128_read_fd(fd, ID128_PLAIN, &machine_id); + r = id128_read_fd(fd, ID128_FORMAT_PLAIN, &machine_id); if (r < 0) log_debug_errno(r, "Image %s contains invalid machine ID.", i->name); } diff --git a/src/shared/machine-id-setup.c b/src/shared/machine-id-setup.c index c694a185ba..132e372e07 100644 --- a/src/shared/machine-id-setup.c +++ b/src/shared/machine-id-setup.c @@ -37,7 +37,7 @@ static int generate_machine_id(const char *root, sd_id128_t *ret) { dbus_machine_id = prefix_roota(root, "/var/lib/dbus/machine-id"); fd = open(dbus_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW); if (fd >= 0) { - if (id128_read_fd(fd, ID128_PLAIN, ret) >= 0) { + if (id128_read_fd(fd, ID128_FORMAT_PLAIN, ret) >= 0) { log_info("Initializing machine ID from D-Bus machine ID."); return 0; } @@ -122,7 +122,7 @@ int machine_id_setup(const char *root, bool force_transient, sd_id128_t machine_ if (sd_id128_is_null(machine_id)) { /* Try to read any existing machine ID */ - if (id128_read_fd(fd, ID128_PLAIN, ret) >= 0) + if (id128_read_fd(fd, ID128_FORMAT_PLAIN, ret) >= 0) return 0; /* Hmm, so, the id currently stored is not useful, then let's generate one */ @@ -151,7 +151,7 @@ int machine_id_setup(const char *root, bool force_transient, sd_id128_t machine_ if (r < 0) return log_error_errno(r, "Failed to sync %s: %m", etc_machine_id); } else { - r = id128_write_fd(fd, ID128_PLAIN, machine_id, true); + r = id128_write_fd(fd, ID128_FORMAT_PLAIN, machine_id, true); if (r < 0) return log_error_errno(r, "Failed to write %s: %m", etc_machine_id); else @@ -167,7 +167,7 @@ int machine_id_setup(const char *root, bool force_transient, sd_id128_t machine_ run_machine_id = prefix_roota(root, "/run/machine-id"); RUN_WITH_UMASK(0022) - r = id128_write(run_machine_id, ID128_PLAIN, machine_id, false); + r = id128_write(run_machine_id, ID128_FORMAT_PLAIN, machine_id, false); if (r < 0) { (void) unlink(run_machine_id); return log_error_errno(r, "Cannot write %s: %m", run_machine_id); @@ -239,7 +239,7 @@ int machine_id_commit(const char *root) { "%s is not on a temporary file system.", etc_machine_id); - r = id128_read_fd(fd, ID128_PLAIN, &id); + r = id128_read_fd(fd, ID128_FORMAT_PLAIN, &id); if (r < 0) return log_error_errno(r, "We didn't find a valid machine ID in %s: %m", etc_machine_id); @@ -260,7 +260,7 @@ int machine_id_commit(const char *root) { return r; /* Update a persistent version of etc_machine_id */ - r = id128_write(etc_machine_id, ID128_PLAIN, id, true); + r = id128_write(etc_machine_id, ID128_FORMAT_PLAIN, id, true); if (r < 0) return log_error_errno(r, "Cannot write %s. This is mandatory to get a persistent machine ID: %m", etc_machine_id); diff --git a/src/shared/specifier.c b/src/shared/specifier.c index d54ab9f5a9..cd651768bd 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -195,7 +195,7 @@ int specifier_machine_id(char specifier, const void *data, const char *root, con /* Translate error for missing os-release file to EUNATCH. */ return fd == -ENOENT ? -EUNATCH : fd; - r = id128_read_fd(fd, ID128_PLAIN, &id); + r = id128_read_fd(fd, ID128_FORMAT_PLAIN, &id); } else r = sd_id128_get_machine(&id); if (r < 0) diff --git a/src/test/test-condition.c b/src/test/test-condition.c index 5144eb4687..901d80eb3d 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -250,7 +250,7 @@ TEST(condition_test_host) { int r; r = sd_id128_get_machine(&id); - if (IN_SET(r, -ENOENT, -ENOMEDIUM)) + if (IN_SET(r, -ENOENT, -ENOMEDIUM, -ENOPKG)) return (void) log_tests_skipped("/etc/machine-id missing"); assert_se(r >= 0); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 8c4b632ed0..16c6774a65 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -325,7 +325,7 @@ TEST(chase_symlinks) { assert_se(fd >= 0); safe_close(pfd); - assert_se(id128_read_fd(fd, ID128_PLAIN, &a) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_PLAIN, &a) >= 0); assert_se(sd_id128_get_machine(&b) >= 0); assert_se(sd_id128_equal(a, b)); } diff --git a/src/test/test-id128.c b/src/test/test-id128.c index 4b71c5c00b..4175ac7b60 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -86,17 +86,17 @@ TEST(id128) { /* First, write as UUID */ assert_se(sd_id128_randomize(&id) >= 0); - assert_se(id128_write_fd(fd, ID128_UUID, id, false) >= 0); + assert_se(id128_write_fd(fd, ID128_FORMAT_UUID, id, false) >= 0); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_PLAIN, &id2) == -EINVAL); + assert_se(id128_read_fd(fd, ID128_FORMAT_PLAIN, &id2) == -EINVAL); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_UUID, &id2) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_UUID, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_ANY, &id2) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_ANY, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); /* Second, write as plain */ @@ -104,17 +104,17 @@ TEST(id128) { assert_se(ftruncate(fd, 0) >= 0); assert_se(sd_id128_randomize(&id) >= 0); - assert_se(id128_write_fd(fd, ID128_PLAIN, id, false) >= 0); + assert_se(id128_write_fd(fd, ID128_FORMAT_PLAIN, id, false) >= 0); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_UUID, &id2) == -EINVAL); + assert_se(id128_read_fd(fd, ID128_FORMAT_UUID, &id2) == -EINVAL); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_PLAIN, &id2) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_PLAIN, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_ANY, &id2) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_ANY, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); /* Third, write plain without trailing newline */ @@ -125,13 +125,13 @@ TEST(id128) { assert_se(write(fd, sd_id128_to_string(id, t), 32) == 32); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_UUID, &id2) == -EINVAL); + assert_se(id128_read_fd(fd, ID128_FORMAT_UUID, &id2) == -EINVAL); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_PLAIN, &id2) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_PLAIN, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); - /* Third, write UUID without trailing newline */ + /* Fourth, write UUID without trailing newline */ assert_se(lseek(fd, 0, SEEK_SET) == 0); assert_se(ftruncate(fd, 0) >= 0); @@ -139,10 +139,10 @@ TEST(id128) { assert_se(write(fd, sd_id128_to_uuid_string(id, q), 36) == 36); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_PLAIN, &id2) == -EINVAL); + assert_se(id128_read_fd(fd, ID128_FORMAT_PLAIN, &id2) == -EINVAL); assert_se(lseek(fd, 0, SEEK_SET) == 0); - assert_se(id128_read_fd(fd, ID128_UUID, &id2) >= 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_UUID, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); if (sd_booted() > 0 && access("/etc/machine-id", F_OK) >= 0) { From 66c7949e08a977a9d0c32dcfadef0bb843956b8d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 15:18:28 +0900 Subject: [PATCH 4/9] test: add tests for "uninitialized" string handling by id128_read_fd() --- src/test/test-id128.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/test-id128.c b/src/test/test-id128.c index 4175ac7b60..dccf3b7fb9 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -145,6 +145,31 @@ TEST(id128) { assert_se(id128_read_fd(fd, ID128_FORMAT_UUID, &id2) >= 0); assert_se(sd_id128_equal(id, id2)); + /* Fifth, tests for "uninitialized" */ + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(ftruncate(fd, 0) >= 0); + assert_se(write(fd, "uninitialized", STRLEN("uninitialized")) == STRLEN("uninitialized")); + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_ANY, NULL) == -ENOPKG); + + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(ftruncate(fd, 0) >= 0); + assert_se(write(fd, "uninitialized\n", STRLEN("uninitialized\n")) == STRLEN("uninitialized\n")); + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_ANY, NULL) == -ENOPKG); + + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(ftruncate(fd, 0) >= 0); + assert_se(write(fd, "uninitialized\nfoo", STRLEN("uninitialized\nfoo")) == STRLEN("uninitialized\nfoo")); + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_ANY, NULL) == -EINVAL); + + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(ftruncate(fd, 0) >= 0); + assert_se(write(fd, "uninit", STRLEN("uninit")) == STRLEN("uninit")); + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(id128_read_fd(fd, ID128_FORMAT_ANY, NULL) == -EINVAL); + if (sd_booted() > 0 && access("/etc/machine-id", F_OK) >= 0) { assert_se(sd_id128_get_machine_app_specific(SD_ID128_MAKE(f0,3d,aa,eb,1c,33,4b,43,a7,32,17,29,44,bf,77,2e), &id) >= 0); assert_se(sd_id128_get_machine_app_specific(SD_ID128_MAKE(f0,3d,aa,eb,1c,33,4b,43,a7,32,17,29,44,bf,77,2e), &id2) >= 0); From a237c6e0b0b294cebc084891a84173a19eb69172 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 15:43:26 +0900 Subject: [PATCH 5/9] man: mention sd_id128_get_machine() or friend may return -ENOPKG --- man/sd_id128_get_machine.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/man/sd_id128_get_machine.xml b/man/sd_id128_get_machine.xml index a778f8a2b0..8bc76b686b 100644 --- a/man/sd_id128_get_machine.xml +++ b/man/sd_id128_get_machine.xml @@ -144,6 +144,14 @@ empty or all zeros. + + -ENOPKG + + Returned by sd_id128_get_machine() and + sd_id128_get_machine_app_specific() when the content of + /etc/machine-id is uninitialized. + + -ENXIO From 9be90c401e16cf04a9cea0b19fdefa7d0a47f056 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 15:48:25 +0900 Subject: [PATCH 6/9] sd-id128: make sd_id128_get_boot() and friend return -ENOMEDIUM --- src/libsystemd/sd-id128/sd-id128.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index a5758e5841..70b1a10c74 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -148,6 +148,9 @@ _public_ int sd_id128_get_boot(sd_id128_t *ret) { r = id128_read("/proc/sys/kernel/random/boot_id", ID128_FORMAT_UUID, &saved_boot_id); if (r < 0) return r; + + if (sd_id128_is_null(saved_boot_id)) + return -ENOMEDIUM; } *ret = saved_boot_id; From e2720340e9fdf3aee2e8998dc72798de50be3630 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 12 Dec 2022 22:03:52 +0900 Subject: [PATCH 7/9] sd-id128: make sd_id128_get_boot() and friend return -ENOSYS when /proc/ is not mounted --- src/libsystemd/sd-id128/sd-id128.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index 70b1a10c74..8f9801ae37 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -15,6 +15,7 @@ #include "macro.h" #include "missing_syscall.h" #include "random-util.h" +#include "stat-util.h" #include "user-util.h" _public_ char *sd_id128_to_string(sd_id128_t id, char s[_SD_ARRAY_STATIC SD_ID128_STRING_MAX]) { @@ -146,6 +147,8 @@ _public_ int sd_id128_get_boot(sd_id128_t *ret) { if (sd_id128_is_null(saved_boot_id)) { r = id128_read("/proc/sys/kernel/random/boot_id", ID128_FORMAT_UUID, &saved_boot_id); + if (r == -ENOENT && proc_mounted() == 0) + return -ENOSYS; if (r < 0) return r; From c576920e673114529c5bfe5ea29891a24a443338 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Dec 2022 15:49:02 +0900 Subject: [PATCH 8/9] man: mention that sd_id128_get_boot() and friend may return -ENOSYS And drop to mention sd_id128_get_boot_app_specific() may return -ENOENT or -ENOMEDIUM. The function does not read /etc/machine-id. But reads a file in the procfs, which is a kind of the kernel API. Hence the failures are caused only when the system has wrong setup. --- man/sd_id128_get_machine.xml | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/man/sd_id128_get_machine.xml b/man/sd_id128_get_machine.xml index 8bc76b686b..dbc6d4885d 100644 --- a/man/sd_id128_get_machine.xml +++ b/man/sd_id128_get_machine.xml @@ -129,19 +129,17 @@ -ENOENT - Returned by sd_id128_get_machine(), - sd_id128_get_machine_app_specific(), and - sd_id128_get_boot_app_specific() when /etc/machine-id is - missing. + Returned by sd_id128_get_machine() and + sd_id128_get_machine_app_specific() when /etc/machine-id + is missing. -ENOMEDIUM - Returned by sd_id128_get_machine(), - sd_id128_get_machine_app_specific(), and - sd_id128_get_boot_app_specific() when /etc/machine-id is - empty or all zeros. + Returned by sd_id128_get_machine() and + sd_id128_get_machine_app_specific() when /etc/machine-id + is empty or all zeros. @@ -152,6 +150,14 @@ /etc/machine-id is uninitialized. + + -ENOSYS + + Returned by sd_id128_get_boot() and + sd_id128_get_boot_app_specific() when /proc/ is not + mounted. + + -ENXIO From b40c8ebdc86b61df03207865b5a75cd37900ea4c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 9 Dec 2022 05:37:12 +0900 Subject: [PATCH 9/9] sd-id128: fold do_sync flag into Id128FormatFlag --- src/libsystemd/sd-id128/id128-util.c | 8 ++++---- src/libsystemd/sd-id128/id128-util.h | 6 ++++-- src/nspawn/nspawn.c | 2 +- src/shared/machine-id-setup.c | 6 +++--- src/test/test-id128.c | 4 ++-- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 2cf8848981..3395ba2e58 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -109,7 +109,7 @@ int id128_read(const char *p, Id128FormatFlag f, sd_id128_t *ret) { return id128_read_fd(fd, f, ret); } -int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id, bool do_sync) { +int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id) { char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ size_t sz; int r; @@ -130,7 +130,7 @@ int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id, bool do_sync) { if (r < 0) return r; - if (do_sync) { + if (FLAGS_SET(f, ID128_SYNC_ON_WRITE)) { r = fsync_full(fd); if (r < 0) return r; @@ -139,14 +139,14 @@ int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id, bool do_sync) { return 0; } -int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id, bool do_sync) { +int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id) { _cleanup_close_ int fd = -1; fd = open(p, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444); if (fd < 0) return -errno; - return id128_write_fd(fd, f, id, do_sync); + return id128_write_fd(fd, f, id); } void id128_hash_func(const sd_id128_t *p, struct siphash *state) { diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index bb237cb6cc..4b91a16bd1 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -14,13 +14,15 @@ typedef enum Id128FormatFlag { ID128_FORMAT_PLAIN = 1 << 0, /* formatted as 32 hex chars as-is */ ID128_FORMAT_UUID = 1 << 1, /* formatted as 36 character uuid string */ ID128_FORMAT_ANY = ID128_FORMAT_PLAIN | ID128_FORMAT_UUID, + + ID128_SYNC_ON_WRITE = 1 << 2, /* Sync the file after write. Used only when writing an ID. */ } Id128FormatFlag; int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret); int id128_read(const char *p, Id128FormatFlag f, sd_id128_t *ret); -int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id, bool do_sync); -int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id, bool do_sync); +int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id); +int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id); void id128_hash_func(const sd_id128_t *p, struct siphash *state); int id128_compare_func(const sd_id128_t *a, const sd_id128_t *b) _pure_; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 0618ab2313..96611058fe 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2196,7 +2196,7 @@ static int setup_boot_id(void) { if (r < 0) return log_error_errno(r, "Failed to generate random boot id: %m"); - r = id128_write(path, ID128_FORMAT_UUID, rnd, false); + r = id128_write(path, ID128_FORMAT_UUID, rnd); if (r < 0) return log_error_errno(r, "Failed to write boot id: %m"); diff --git a/src/shared/machine-id-setup.c b/src/shared/machine-id-setup.c index 132e372e07..2e33a23cb9 100644 --- a/src/shared/machine-id-setup.c +++ b/src/shared/machine-id-setup.c @@ -151,7 +151,7 @@ int machine_id_setup(const char *root, bool force_transient, sd_id128_t machine_ if (r < 0) return log_error_errno(r, "Failed to sync %s: %m", etc_machine_id); } else { - r = id128_write_fd(fd, ID128_FORMAT_PLAIN, machine_id, true); + r = id128_write_fd(fd, ID128_FORMAT_PLAIN | ID128_SYNC_ON_WRITE, machine_id); if (r < 0) return log_error_errno(r, "Failed to write %s: %m", etc_machine_id); else @@ -167,7 +167,7 @@ int machine_id_setup(const char *root, bool force_transient, sd_id128_t machine_ run_machine_id = prefix_roota(root, "/run/machine-id"); RUN_WITH_UMASK(0022) - r = id128_write(run_machine_id, ID128_FORMAT_PLAIN, machine_id, false); + r = id128_write(run_machine_id, ID128_FORMAT_PLAIN, machine_id); if (r < 0) { (void) unlink(run_machine_id); return log_error_errno(r, "Cannot write %s: %m", run_machine_id); @@ -260,7 +260,7 @@ int machine_id_commit(const char *root) { return r; /* Update a persistent version of etc_machine_id */ - r = id128_write(etc_machine_id, ID128_FORMAT_PLAIN, id, true); + r = id128_write(etc_machine_id, ID128_FORMAT_PLAIN | ID128_SYNC_ON_WRITE, id); if (r < 0) return log_error_errno(r, "Cannot write %s. This is mandatory to get a persistent machine ID: %m", etc_machine_id); diff --git a/src/test/test-id128.c b/src/test/test-id128.c index dccf3b7fb9..6de0cec426 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -86,7 +86,7 @@ TEST(id128) { /* First, write as UUID */ assert_se(sd_id128_randomize(&id) >= 0); - assert_se(id128_write_fd(fd, ID128_FORMAT_UUID, id, false) >= 0); + assert_se(id128_write_fd(fd, ID128_FORMAT_UUID, id) >= 0); assert_se(lseek(fd, 0, SEEK_SET) == 0); assert_se(id128_read_fd(fd, ID128_FORMAT_PLAIN, &id2) == -EINVAL); @@ -104,7 +104,7 @@ TEST(id128) { assert_se(ftruncate(fd, 0) >= 0); assert_se(sd_id128_randomize(&id) >= 0); - assert_se(id128_write_fd(fd, ID128_FORMAT_PLAIN, id, false) >= 0); + assert_se(id128_write_fd(fd, ID128_FORMAT_PLAIN, id) >= 0); assert_se(lseek(fd, 0, SEEK_SET) == 0); assert_se(id128_read_fd(fd, ID128_FORMAT_UUID, &id2) == -EINVAL);