From 17f9d6d806ac59161ebd78afe4979e2c6138545f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 3 Apr 2023 10:39:00 +0900 Subject: [PATCH 1/6] Revert "sd-id128: make id128_read() optionally take root directory" This reverts commit 830e52caa2bf1a29f56cb93e7ed85acb1bda11c3. --- src/libsystemd/sd-id128/id128-util.c | 13 +++++-------- src/libsystemd/sd-id128/id128-util.h | 2 +- src/libsystemd/sd-id128/sd-id128.c | 4 ++-- src/machine-id-setup/machine-id-setup-main.c | 5 ++++- src/nspawn/nspawn.c | 5 ++++- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 974aafa5b7..a009a110a9 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -4,7 +4,6 @@ #include #include -#include "chase.h" #include "fd-util.h" #include "hexdecoct.h" #include "id128-util.h" @@ -102,14 +101,12 @@ int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret) { return r == -EINVAL ? -EUCLEAN : r; } -int id128_read(const char *root, const char *p, Id128FormatFlag f, sd_id128_t *ret) { +int id128_read(const char *p, Id128FormatFlag f, sd_id128_t *ret) { _cleanup_close_ int fd = -EBADF; - assert(p); - - fd = chase_and_open(p, root, CHASE_PREFIX_ROOT, O_RDONLY|O_CLOEXEC|O_NOCTTY, /* ret_path = */ NULL); + fd = open(p, O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) - return fd; + return -errno; return id128_read_fd(fd, f, ret); } @@ -187,9 +184,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(NULL, "/sys/class/dmi/id/product_uuid", ID128_FORMAT_UUID, &uuid); + r = id128_read("/sys/class/dmi/id/product_uuid", ID128_FORMAT_UUID, &uuid); if (r == -ENOENT) - r = id128_read(NULL, "/proc/device-tree/vm,uuid", ID128_FORMAT_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 2ebca8fd95..e094de6441 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -19,7 +19,7 @@ typedef enum Id128FormatFlag { } Id128FormatFlag; int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret); -int id128_read(const char *root, const char *p, 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); int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id); diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index 03f0ddd182..5ce36cf2fc 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) { int r; if (sd_id128_is_null(saved_machine_id)) { - r = id128_read(NULL, "/etc/machine-id", ID128_FORMAT_PLAIN, &saved_machine_id); + r = id128_read("/etc/machine-id", ID128_FORMAT_PLAIN, &saved_machine_id); if (r < 0) return r; @@ -144,7 +144,7 @@ _public_ int sd_id128_get_boot(sd_id128_t *ret) { int r; if (sd_id128_is_null(saved_boot_id)) { - r = id128_read(NULL, "/proc/sys/kernel/random/boot_id", ID128_FORMAT_UUID, &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) diff --git a/src/machine-id-setup/machine-id-setup-main.c b/src/machine-id-setup/machine-id-setup-main.c index 2d29890864..5620bf9baf 100644 --- a/src/machine-id-setup/machine-id-setup-main.c +++ b/src/machine-id-setup/machine-id-setup-main.c @@ -158,11 +158,14 @@ static int run(int argc, char *argv[]) { } if (arg_commit) { + const char *etc_machine_id; + r = machine_id_commit(arg_root); if (r < 0) return r; - r = id128_read(arg_root, "/etc/machine-id", ID128_FORMAT_PLAIN, &id); + etc_machine_id = prefix_roota(arg_root, "/etc/machine-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 982dffd1b8..9fbacb2713 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2828,6 +2828,7 @@ static int mount_tunnel_open(void) { } static int setup_machine_id(const char *directory) { + const char *etc_machine_id; sd_id128_t id; int r; @@ -2838,7 +2839,9 @@ static int setup_machine_id(const char *directory) { * in the container and our idea of the container UUID will always be in sync (at least if PID 1 in the * container behaves nicely). */ - r = id128_read(directory, "/etc/machine-id", ID128_FORMAT_PLAIN, &id); + etc_machine_id = prefix_roota(directory, "/etc/machine-id"); + + r = id128_read(etc_machine_id, ID128_FORMAT_PLAIN, &id); if (r < 0) { if (!ERRNO_IS_MACHINE_ID_UNSET(r)) /* 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"); From 169d91b7276de4b05defb58700fb465fb53cf52a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 2 Apr 2023 00:35:33 +0900 Subject: [PATCH 2/6] sd-id128: rename argument and add missing assertion --- src/libsystemd/sd-id128/id128-util.c | 12 ++++++++---- src/libsystemd/sd-id128/id128-util.h | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index a009a110a9..ab4108204e 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -101,10 +101,12 @@ int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret) { return r == -EINVAL ? -EUCLEAN : r; } -int id128_read(const char *p, Id128FormatFlag f, sd_id128_t *ret) { +int id128_read(const char *path, Id128FormatFlag f, sd_id128_t *ret) { _cleanup_close_ int fd = -EBADF; - fd = open(p, O_RDONLY|O_CLOEXEC|O_NOCTTY); + assert(path); + + fd = open(path, O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) return -errno; @@ -141,10 +143,12 @@ int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id) { return 0; } -int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id) { +int id128_write(const char *path, Id128FormatFlag f, sd_id128_t id) { _cleanup_close_ int fd = -EBADF; - fd = open(p, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444); + assert(path); + + fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444); if (fd < 0) return -errno; diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index e094de6441..4be10bb310 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -19,10 +19,10 @@ typedef enum Id128FormatFlag { } 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_read(const char *path, Id128FormatFlag f, sd_id128_t *ret); int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id); -int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id); +int id128_write(const char *path, 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_; From 762b78dee91b7984d8d8dd57b159372a1c45fce6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Apr 2023 21:30:45 +0900 Subject: [PATCH 3/6] sd-id128: rename Id128FormatFlag -> Id128Flag As the flag is not only for formatting. --- src/libsystemd/sd-id128/id128-util.c | 8 ++++---- src/libsystemd/sd-id128/id128-util.h | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index ab4108204e..0d77b04735 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -40,7 +40,7 @@ bool id128_is_valid(const char *s) { return false; } -int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret) { +int id128_read_fd(int fd, Id128Flag f, sd_id128_t *ret) { char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ ssize_t l; int r; @@ -101,7 +101,7 @@ int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret) { return r == -EINVAL ? -EUCLEAN : r; } -int id128_read(const char *path, Id128FormatFlag f, sd_id128_t *ret) { +int id128_read(const char *path, Id128Flag f, sd_id128_t *ret) { _cleanup_close_ int fd = -EBADF; assert(path); @@ -113,7 +113,7 @@ int id128_read(const char *path, 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) { +int id128_write_fd(int fd, Id128Flag f, sd_id128_t id) { char buffer[SD_ID128_UUID_STRING_MAX + 1]; /* +1 is for trailing newline */ size_t sz; int r; @@ -143,7 +143,7 @@ int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id) { return 0; } -int id128_write(const char *path, Id128FormatFlag f, sd_id128_t id) { +int id128_write(const char *path, Id128Flag f, sd_id128_t id) { _cleanup_close_ int fd = -EBADF; assert(path); diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index 4be10bb310..834fa4cd75 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -10,19 +10,19 @@ bool id128_is_valid(const char *s) _pure_; -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, +typedef enum Id128Flag { + 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; +} Id128Flag; -int id128_read_fd(int fd, Id128FormatFlag f, sd_id128_t *ret); -int id128_read(const char *path, Id128FormatFlag f, sd_id128_t *ret); +int id128_read_fd(int fd, Id128Flag f, sd_id128_t *ret); +int id128_read(const char *path, Id128Flag f, sd_id128_t *ret); -int id128_write_fd(int fd, Id128FormatFlag f, sd_id128_t id); -int id128_write(const char *path, Id128FormatFlag f, sd_id128_t id); +int id128_write_fd(int fd, Id128Flag f, sd_id128_t id); +int id128_write(const char *path, Id128Flag 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_; From 0f44b76601d6ca31a634597ce79304efa5af0153 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 31 Mar 2023 16:10:36 +0900 Subject: [PATCH 4/6] sd-id128: introduce id128_read_at() --- src/libsystemd/sd-id128/id128-util.c | 8 +++++--- src/libsystemd/sd-id128/id128-util.h | 6 +++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 0d77b04735..94094ed208 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -5,6 +5,7 @@ #include #include "fd-util.h" +#include "fs-util.h" #include "hexdecoct.h" #include "id128-util.h" #include "io-util.h" @@ -101,14 +102,15 @@ int id128_read_fd(int fd, Id128Flag f, sd_id128_t *ret) { return r == -EINVAL ? -EUCLEAN : r; } -int id128_read(const char *path, Id128Flag f, sd_id128_t *ret) { +int id128_read_at(int dir_fd, const char *path, Id128Flag f, sd_id128_t *ret) { _cleanup_close_ int fd = -EBADF; + assert(dir_fd >= 0 || dir_fd == AT_FDCWD); assert(path); - fd = open(path, O_RDONLY|O_CLOEXEC|O_NOCTTY); + fd = xopenat(dir_fd, path, O_RDONLY|O_CLOEXEC|O_NOCTTY, 0); if (fd < 0) - return -errno; + return fd; return id128_read_fd(fd, f, ret); } diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index 834fa4cd75..7bab3e8303 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include #include #include "sd-id128.h" @@ -19,7 +20,10 @@ typedef enum Id128Flag { } Id128Flag; int id128_read_fd(int fd, Id128Flag f, sd_id128_t *ret); -int id128_read(const char *path, Id128Flag f, sd_id128_t *ret); +int id128_read_at(int dir_fd, const char *path, Id128Flag f, sd_id128_t *ret); +static inline int id128_read(const char *path, Id128Flag f, sd_id128_t *ret) { + return id128_read_at(AT_FDCWD, path, f, ret); +} int id128_write_fd(int fd, Id128Flag f, sd_id128_t id); int id128_write(const char *path, Id128Flag f, sd_id128_t id); From c227c46b8b6fefe2604ca110b2e2c4e4f78c2c9d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 31 Mar 2023 16:12:39 +0900 Subject: [PATCH 5/6] sd-id128: introduce id128_write_at() --- src/libsystemd/sd-id128/id128-util.c | 7 ++++--- src/libsystemd/sd-id128/id128-util.h | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 94094ed208..3fa1585aa3 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -145,14 +145,15 @@ int id128_write_fd(int fd, Id128Flag f, sd_id128_t id) { return 0; } -int id128_write(const char *path, Id128Flag f, sd_id128_t id) { +int id128_write_at(int dir_fd, const char *path, Id128Flag f, sd_id128_t id) { _cleanup_close_ int fd = -EBADF; + assert(dir_fd >= 0 || dir_fd == AT_FDCWD); assert(path); - fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444); + fd = xopenat(dir_fd, path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444); if (fd < 0) - return -errno; + return fd; return id128_write_fd(fd, f, id); } diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index 7bab3e8303..fde58a5228 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -26,7 +26,10 @@ static inline int id128_read(const char *path, Id128Flag f, sd_id128_t *ret) { } int id128_write_fd(int fd, Id128Flag f, sd_id128_t id); -int id128_write(const char *path, Id128Flag f, sd_id128_t id); +int id128_write_at(int dir_fd, const char *path, Id128Flag f, sd_id128_t id); +static inline int id128_write(const char *path, Id128Flag f, sd_id128_t id) { + return id128_write_at(AT_FDCWD, path, f, 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_; From 9853542dd396ff7c294b21aed72dc9316152b6e7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 4 Apr 2023 11:39:33 +0900 Subject: [PATCH 6/6] test: add tests for id128_{read,write}_at() --- src/test/test-id128.c | 74 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/test/test-id128.c b/src/test/test-id128.c index 0a97366b6d..75c3582734 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -11,6 +11,8 @@ #include "fd-util.h" #include "id128-util.h" #include "macro.h" +#include "path-util.h" +#include "rm-rf.h" #include "string-util.h" #include "tests.h" #include "tmpfile-util.h" @@ -215,4 +217,76 @@ TEST(benchmark_sd_id128_get_machine_app_specific) { log_info("%lf µs each\n", (double) q / iterations); } +TEST(id128_at) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_close_ int tfd = -EBADF; + _cleanup_free_ char *p = NULL; + sd_id128_t id, i; + + tfd = mkdtemp_open(NULL, O_PATH, &t); + assert_se(tfd >= 0); + assert_se(mkdirat(tfd, "etc", 0755) >= 0); + assert_se(symlinkat("etc", tfd, "etc2") >= 0); + assert_se(symlinkat("machine-id", tfd, "etc/hoge-id") >= 0); + + assert_se(sd_id128_randomize(&id) == 0); + + assert_se(id128_write_at(tfd, "etc/machine-id", ID128_FORMAT_PLAIN, id) >= 0); + if (geteuid() == 0) + assert_se(id128_write_at(tfd, "etc/machine-id", ID128_FORMAT_PLAIN, id) >= 0); + else + assert_se(id128_write_at(tfd, "etc/machine-id", ID128_FORMAT_PLAIN, id) == -EACCES); + assert_se(unlinkat(tfd, "etc/machine-id", 0) >= 0); + assert_se(id128_write_at(tfd, "etc2/machine-id", ID128_FORMAT_PLAIN, id) >= 0); + assert_se(unlinkat(tfd, "etc/machine-id", 0) >= 0); + assert_se(id128_write_at(tfd, "etc/hoge-id", ID128_FORMAT_PLAIN, id) >= 0); + assert_se(unlinkat(tfd, "etc/machine-id", 0) >= 0); + assert_se(id128_write_at(tfd, "etc2/hoge-id", ID128_FORMAT_PLAIN, id) >= 0); + + /* id128_read_at() */ + i = SD_ID128_NULL; /* Not necessary in real code, but for testing that the id is really assigned. */ + assert_se(id128_read_at(tfd, "etc/machine-id", ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + i = SD_ID128_NULL; + assert_se(id128_read_at(tfd, "etc2/machine-id", ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + i = SD_ID128_NULL; + assert_se(id128_read_at(tfd, "etc/hoge-id", ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + i = SD_ID128_NULL; + assert_se(id128_read_at(tfd, "etc2/hoge-id", ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + /* id128_read() */ + assert_se(p = path_join(t, "/etc/machine-id")); + + i = SD_ID128_NULL; + assert_se(id128_read(p, ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + free(p); + assert_se(p = path_join(t, "/etc2/machine-id")); + + i = SD_ID128_NULL; + assert_se(id128_read(p, ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + free(p); + assert_se(p = path_join(t, "/etc/hoge-id")); + + i = SD_ID128_NULL; + assert_se(id128_read(p, ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); + + free(p); + assert_se(p = path_join(t, "/etc2/hoge-id")); + + i = SD_ID128_NULL; + assert_se(id128_read(p, ID128_FORMAT_PLAIN, &i) >= 0); + assert_se(sd_id128_equal(id, i)); +} + DEFINE_TEST_MAIN(LOG_INFO);