From 5178d4a77e4d556a0946b0701c5228b216bff7cf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 04:20:06 +0900 Subject: [PATCH 01/14] namespace-util: rename arguments to prefix 'ret_' Also reorder opening namespace fd to make it match with the order of the arguments. --- src/basic/namespace-util.c | 75 +++++++++++++++++++++----------------- src/basic/namespace-util.h | 8 +++- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/basic/namespace-util.c b/src/basic/namespace-util.c index 2101f617ad..501d2fcaf3 100644 --- a/src/basic/namespace-util.c +++ b/src/basic/namespace-util.c @@ -33,71 +33,78 @@ const struct namespace_info namespace_info[] = { #define pid_namespace_path(pid, type) procfs_file_alloca(pid, namespace_info[type].proc_path) -int namespace_open(pid_t pid, int *pidns_fd, int *mntns_fd, int *netns_fd, int *userns_fd, int *root_fd) { - _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, netnsfd = -EBADF, usernsfd = -EBADF; - int rfd = -EBADF; +int namespace_open( + pid_t pid, + int *ret_pidns_fd, + int *ret_mntns_fd, + int *ret_netns_fd, + int *ret_userns_fd, + int *ret_root_fd) { + + _cleanup_close_ int pidns_fd = -EBADF, mntns_fd = -EBADF, netns_fd = -EBADF, + userns_fd = -EBADF, root_fd = -EBADF; assert(pid >= 0); - if (mntns_fd) { - const char *mntns; - - mntns = pid_namespace_path(pid, NAMESPACE_MOUNT); - mntnsfd = open(mntns, O_RDONLY|O_NOCTTY|O_CLOEXEC); - if (mntnsfd < 0) - return -errno; - } - - if (pidns_fd) { + if (ret_pidns_fd) { const char *pidns; pidns = pid_namespace_path(pid, NAMESPACE_PID); - pidnsfd = open(pidns, O_RDONLY|O_NOCTTY|O_CLOEXEC); - if (pidnsfd < 0) + pidns_fd = open(pidns, O_RDONLY|O_NOCTTY|O_CLOEXEC); + if (pidns_fd < 0) return -errno; } - if (netns_fd) { + if (ret_mntns_fd) { + const char *mntns; + + mntns = pid_namespace_path(pid, NAMESPACE_MOUNT); + mntns_fd = open(mntns, O_RDONLY|O_NOCTTY|O_CLOEXEC); + if (mntns_fd < 0) + return -errno; + } + + if (ret_netns_fd) { const char *netns; netns = pid_namespace_path(pid, NAMESPACE_NET); - netnsfd = open(netns, O_RDONLY|O_NOCTTY|O_CLOEXEC); - if (netnsfd < 0) + netns_fd = open(netns, O_RDONLY|O_NOCTTY|O_CLOEXEC); + if (netns_fd < 0) return -errno; } - if (userns_fd) { + if (ret_userns_fd) { const char *userns; userns = pid_namespace_path(pid, NAMESPACE_USER); - usernsfd = open(userns, O_RDONLY|O_NOCTTY|O_CLOEXEC); - if (usernsfd < 0 && errno != ENOENT) + userns_fd = open(userns, O_RDONLY|O_NOCTTY|O_CLOEXEC); + if (userns_fd < 0 && errno != ENOENT) return -errno; } - if (root_fd) { + if (ret_root_fd) { const char *root; root = procfs_file_alloca(pid, "root"); - rfd = open(root, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY); - if (rfd < 0) + root_fd = open(root, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY); + if (root_fd < 0) return -errno; } - if (pidns_fd) - *pidns_fd = TAKE_FD(pidnsfd); + if (ret_pidns_fd) + *ret_pidns_fd = TAKE_FD(pidns_fd); - if (mntns_fd) - *mntns_fd = TAKE_FD(mntnsfd); + if (ret_mntns_fd) + *ret_mntns_fd = TAKE_FD(mntns_fd); - if (netns_fd) - *netns_fd = TAKE_FD(netnsfd); + if (ret_netns_fd) + *ret_netns_fd = TAKE_FD(netns_fd); - if (userns_fd) - *userns_fd = TAKE_FD(usernsfd); + if (ret_userns_fd) + *ret_userns_fd = TAKE_FD(userns_fd); - if (root_fd) - *root_fd = TAKE_FD(rfd); + if (ret_root_fd) + *ret_root_fd = TAKE_FD(root_fd); return 0; } diff --git a/src/basic/namespace-util.h b/src/basic/namespace-util.h index be5b2281d3..cd5b8ecab5 100644 --- a/src/basic/namespace-util.h +++ b/src/basic/namespace-util.h @@ -22,7 +22,13 @@ extern const struct namespace_info { unsigned int clone_flag; } namespace_info[_NAMESPACE_TYPE_MAX + 1]; -int namespace_open(pid_t pid, int *pidns_fd, int *mntns_fd, int *netns_fd, int *userns_fd, int *root_fd); +int namespace_open( + pid_t pid, + int *ret_pidns_fd, + int *ret_mntns_fd, + int *ret_netns_fd, + int *ret_userns_fd, + int *ret_root_fd); int namespace_enter(int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd); int fd_is_ns(int fd, unsigned long nsflag); From d2881ef96ec63c29a4ed41a23133068c821c4fb0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 19 Jan 2024 10:46:20 +0900 Subject: [PATCH 02/14] tree-wide: add short comments for namespace_open() and namespace_enter() Also use -EBADF when unspecified. --- src/basic/namespace-util.c | 7 +++++- src/basic/terminal-util.c | 4 ++-- src/libsystemd/sd-bus/bus-container.c | 2 +- src/machine/machine-dbus.c | 21 ++++++++++++++--- src/nspawn/nspawn-mount.c | 19 ++++++++++++--- src/nspawn/nspawn.c | 33 +++++++++++++++++++++++---- src/shared/logs-show.c | 2 +- src/shared/machine-id-setup.c | 13 +++++++++-- src/shared/mount-util.c | 2 +- src/shared/socket-netlink.c | 8 +++---- 10 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/basic/namespace-util.c b/src/basic/namespace-util.c index 501d2fcaf3..0e81b1de6f 100644 --- a/src/basic/namespace-util.c +++ b/src/basic/namespace-util.c @@ -243,7 +243,12 @@ int userns_acquire(const char *uid_map, const char *gid_map) { if (r < 0) return log_error_errno(r, "Failed to write GID map: %m"); - r = namespace_open(pid, NULL, NULL, NULL, &userns_fd, NULL); + r = namespace_open(pid, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, + /* ret_netns_fd = */ NULL, + &userns_fd, + /* ret_root_fd = */ NULL); if (r < 0) return log_error_errno(r, "Failed to open userns fd: %m"); diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 488541ed23..4c1824bc83 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1211,7 +1211,7 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) { assert(pid > 0); - r = namespace_open(pid, &pidnsfd, &mntnsfd, NULL, &usernsfd, &rootfd); + r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd = */ NULL, &usernsfd, &rootfd); if (r < 0) return r; @@ -1262,7 +1262,7 @@ int open_terminal_in_namespace(pid_t pid, const char *name, int mode) { pid_t child; int r; - r = namespace_open(pid, &pidnsfd, &mntnsfd, NULL, &usernsfd, &rootfd); + r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd = */ NULL, &usernsfd, &rootfd); if (r < 0) return r; diff --git a/src/libsystemd/sd-bus/bus-container.c b/src/libsystemd/sd-bus/bus-container.c index 4146a6efd3..2eca82b033 100644 --- a/src/libsystemd/sd-bus/bus-container.c +++ b/src/libsystemd/sd-bus/bus-container.c @@ -34,7 +34,7 @@ int bus_container_connect_socket(sd_bus *b) { log_debug("sd-bus: connecting bus%s%s to namespace of PID "PID_FMT"...", b->description ? " " : "", strempty(b->description), b->nspid); - r = namespace_open(b->nspid, &pidnsfd, &mntnsfd, NULL, &usernsfd, &rootfd); + r = namespace_open(b->nspid, &pidnsfd, &mntnsfd, /* ret_netns_fd = */ NULL, &usernsfd, &rootfd); if (r < 0) return log_debug_errno(r, "Failed to open namespace of PID "PID_FMT": %m", b->nspid); diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 6c2c2232fe..a7a53eacaf 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -232,7 +232,12 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd if (streq(us, them)) return sd_bus_error_setf(error, BUS_ERROR_NO_PRIVATE_NETWORKING, "Machine %s does not use private networking", m->name); - r = namespace_open(m->leader.pid, NULL, NULL, &netns_fd, NULL, NULL); + r = namespace_open(m->leader.pid, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, + &netns_fd, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) return r; @@ -366,7 +371,12 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s _cleanup_fclose_ FILE *f = NULL; pid_t child; - r = namespace_open(m->leader.pid, &pidns_fd, &mntns_fd, NULL, NULL, &root_fd); + r = namespace_open(m->leader.pid, + &pidns_fd, + &mntns_fd, + /* ret_netns_fd = */ NULL, + /* ret_userns_fd = */ NULL, + &root_fd); if (r < 0) return r; @@ -1069,7 +1079,12 @@ int bus_machine_method_open_root_directory(sd_bus_message *message, void *userda _cleanup_close_pair_ int pair[2] = EBADF_PAIR; pid_t child; - r = namespace_open(m->leader.pid, NULL, &mntns_fd, NULL, NULL, &root_fd); + r = namespace_open(m->leader.pid, + /* ret_pidns_fd = */ NULL, + &mntns_fd, + /* ret_netns_fd = */ NULL, + /* ret_userns_fd = */ NULL, + &root_fd); if (r < 0) return r; diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 470f477f22..24771076b4 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -1388,17 +1388,30 @@ int wipe_fully_visible_fs(int mntns_fd) { _cleanup_close_ int orig_mntns_fd = -EBADF; int r, rr; - r = namespace_open(0, NULL, &orig_mntns_fd, NULL, NULL, NULL); + r = namespace_open(0, + /* ret_pidns_fd = */ NULL, + &orig_mntns_fd, + /* ret_netns_fd = */ NULL, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) return log_error_errno(r, "Failed to pin originating mount namespace: %m"); - r = namespace_enter(-EBADF, mntns_fd, -EBADF, -EBADF, -EBADF); + r = namespace_enter(/* pidns_fd = */ -EBADF, + mntns_fd, + /* netns_fd = */ -EBADF, + /* userns_fd = */ -EBADF, + /* root_fd = */ -EBADF); if (r < 0) return log_error_errno(r, "Failed to enter mount namespace: %m"); rr = do_wipe_fully_visible_fs(); - r = namespace_enter(-EBADF, orig_mntns_fd, -EBADF, -EBADF, -EBADF); + r = namespace_enter(/* pidns_fd = */ -EBADF, + orig_mntns_fd, + /* netns_fd = */ -EBADF, + /* userns_fd = */ -EBADF, + /* root_fd = */ -EBADF); if (r < 0) return log_error_errno(r, "Failed to enter original mount namespace: %m"); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 7ec9889870..5dd3ff5549 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3781,7 +3781,12 @@ static int outer_child( return r; if (arg_userns_mode != USER_NAMESPACE_NO) { - r = namespace_open(0, NULL, &mntns_fd, NULL, NULL, NULL); + r = namespace_open(0, + /* ret_pidns_fd = */ NULL, + &mntns_fd, + /* ret_netns_fd = */ NULL, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) return log_error_errno(r, "Failed to pin outer mount namespace: %m"); @@ -4130,7 +4135,11 @@ static int outer_child( * user if user namespaces are turned on. */ if (arg_network_namespace_path) { - r = namespace_enter(-1, -1, netns_fd, -1, -1); + r = namespace_enter(/* pidns_fd = */ -EBADF, + /* mntns_fd = */ -EBADF, + netns_fd, + /* userns_fd = */ -EBADF, + /* root_fd = */ -EBADF); if (r < 0) return log_error_errno(r, "Failed to join network namespace: %m"); } @@ -5078,7 +5087,12 @@ static int run_container( if (child_netns_fd < 0) { /* Make sure we have an open file descriptor to the child's network * namespace so it stays alive even if the child exits. */ - r = namespace_open(*pid, NULL, NULL, &child_netns_fd, NULL, NULL); + r = namespace_open(*pid, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, + &child_netns_fd, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) return log_error_errno(r, "Failed to open child network namespace: %m"); } @@ -5372,13 +5386,22 @@ static int run_container( if (r == 0) { _cleanup_close_ int parent_netns_fd = -EBADF; - r = namespace_open(getpid_cached(), NULL, NULL, &parent_netns_fd, NULL, NULL); + r = namespace_open(0, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, + &parent_netns_fd, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) { log_error_errno(r, "Failed to open parent network namespace: %m"); _exit(EXIT_FAILURE); } - r = namespace_enter(-1, -1, child_netns_fd, -1, -1); + r = namespace_enter(/* pidns_fd = */ -EBADF, + /* mntns_fd = */ -EBADF, + child_netns_fd, + /* userns_fd = */ -EBADF, + /* root_fd = */ -EBADF); if (r < 0) { log_error_errno(r, "Failed to enter child network namespace: %m"); _exit(EXIT_FAILURE); diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 858b707f6c..1f0279cde3 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1688,7 +1688,7 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { if (r < 0) return r; - r = namespace_open(pid, &pidnsfd, &mntnsfd, NULL, NULL, &rootfd); + r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd = */ NULL, /* ret_userns_fd = */ NULL, &rootfd); if (r < 0) return r; diff --git a/src/shared/machine-id-setup.c b/src/shared/machine-id-setup.c index d6aa667ada..833c98b88e 100644 --- a/src/shared/machine-id-setup.c +++ b/src/shared/machine-id-setup.c @@ -265,7 +265,12 @@ int machine_id_commit(const char *root) { fd = safe_close(fd); /* Store current mount namespace */ - r = namespace_open(0, NULL, &initial_mntns_fd, NULL, NULL, NULL); + r = namespace_open(0, + /* ret_pidns_fd = */ NULL, + &initial_mntns_fd, + /* ret_netns_fd = */ NULL, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) return log_error_errno(r, "Can't fetch current mount namespace: %m"); @@ -284,7 +289,11 @@ int machine_id_commit(const char *root) { return log_error_errno(r, "Cannot write %s. This is mandatory to get a persistent machine ID: %m", etc_machine_id); /* Return to initial namespace and proceed a lazy tmpfs unmount */ - r = namespace_enter(-1, initial_mntns_fd, -1, -1, -1); + r = namespace_enter(/* pidns_fd = */ -EBADF, + initial_mntns_fd, + /* netns_fd = */ -EBADF, + /* userns_fd = */ -EBADF, + /* root_fd = */ -EBADF); if (r < 0) return log_warning_errno(r, "Failed to switch back to initial mount namespace: %m.\nWe'll keep transient %s file until next reboot.", etc_machine_id); diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index 3305b6360e..bf351820a7 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1100,7 +1100,7 @@ static int mount_in_namespace( if (!pidref_is_set(target)) return -ESRCH; - r = namespace_open(target->pid, &pidns_fd, &mntns_fd, NULL, NULL, &root_fd); + r = namespace_open(target->pid, &pidns_fd, &mntns_fd, /* ret_netns_fd = */ NULL, /* ret_userns_fd = */ NULL, &root_fd); if (r < 0) return log_debug_errno(r, "Failed to retrieve FDs of the target process' namespace: %m"); diff --git a/src/shared/socket-netlink.c b/src/shared/socket-netlink.c index 94b699a5ee..6865b045b5 100644 --- a/src/shared/socket-netlink.c +++ b/src/shared/socket-netlink.c @@ -420,11 +420,11 @@ int netns_get_nsid(int netnsfd, uint32_t *ret) { if (netnsfd < 0) { r = namespace_open( 0, - /* pidns_fd= */ NULL, - /* mntns_fd= */ NULL, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, &_netns_fd, - /* userns_fd= */ NULL, - /* root_fd= */ NULL); + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); if (r < 0) return r; From cb566b5d13642cf960c490fd28235a4a504a8b8f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 19 Jan 2024 11:30:16 +0900 Subject: [PATCH 03/14] namespace-util: downgrade log level in userns_acquire() On failure, the caller logs the error in LOG_ERR. --- src/basic/namespace-util.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/basic/namespace-util.c b/src/basic/namespace-util.c index 0e81b1de6f..2ecce5fa61 100644 --- a/src/basic/namespace-util.c +++ b/src/basic/namespace-util.c @@ -228,7 +228,7 @@ int userns_acquire(const char *uid_map, const char *gid_map) { r = safe_fork("(sd-mkuserns)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGKILL|FORK_NEW_USERNS, &pid); if (r < 0) - return r; + return log_debug_errno(r, "Failed to fork process (sd-mkuserns): %m"); if (r == 0) /* Child. We do nothing here, just freeze until somebody kills us. */ freeze(); @@ -236,12 +236,12 @@ int userns_acquire(const char *uid_map, const char *gid_map) { xsprintf(path, "/proc/" PID_FMT "/uid_map", pid); r = write_string_file(path, uid_map, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) - return log_error_errno(r, "Failed to write UID map: %m"); + return log_debug_errno(r, "Failed to write UID map: %m"); xsprintf(path, "/proc/" PID_FMT "/gid_map", pid); r = write_string_file(path, gid_map, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) - return log_error_errno(r, "Failed to write GID map: %m"); + return log_debug_errno(r, "Failed to write GID map: %m"); r = namespace_open(pid, /* ret_pidns_fd = */ NULL, @@ -250,10 +250,9 @@ int userns_acquire(const char *uid_map, const char *gid_map) { &userns_fd, /* ret_root_fd = */ NULL); if (r < 0) - return log_error_errno(r, "Failed to open userns fd: %m"); + return log_debug_errno(r, "Failed to open userns fd: %m"); return TAKE_FD(userns_fd); - } int in_same_namespace(pid_t pid1, pid_t pid2, NamespaceType type) { From 387f39ea30def0aa9a14ee216b46f787241ac7fb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 19 Jan 2024 11:32:10 +0900 Subject: [PATCH 04/14] process-util: introduce FORK_NEW_NETNS for safe_fork() Similar to FORK_NEW_MOUNTNS or FORK_NEW_USERNS. --- src/basic/process-util.c | 5 +++-- src/basic/process-util.h | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index d75d25af99..697c8d9c6b 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1541,10 +1541,11 @@ int safe_fork_full( } } - if ((flags & (FORK_NEW_MOUNTNS|FORK_NEW_USERNS)) != 0) + if ((flags & (FORK_NEW_MOUNTNS|FORK_NEW_USERNS|FORK_NEW_NETNS)) != 0) pid = raw_clone(SIGCHLD| (FLAGS_SET(flags, FORK_NEW_MOUNTNS) ? CLONE_NEWNS : 0) | - (FLAGS_SET(flags, FORK_NEW_USERNS) ? CLONE_NEWUSER : 0)); + (FLAGS_SET(flags, FORK_NEW_USERNS) ? CLONE_NEWUSER : 0) | + (FLAGS_SET(flags, FORK_NEW_NETNS) ? CLONE_NEWNET : 0)); else pid = fork(); if (pid < 0) diff --git a/src/basic/process-util.h b/src/basic/process-util.h index de6a2bd203..b270fc82ea 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -154,11 +154,11 @@ int must_be_root(void); pid_t clone_with_nested_stack(int (*fn)(void *), int flags, void *userdata); -/* 💣 Note that FORK_NEW_USERNS + FORK_NEW_MOUNTNS should not be called in threaded programs, because they - * cause us to use raw_clone() which does not synchronize the glibc malloc() locks, and thus will cause - * deadlocks if the parent uses threads and the child does memory allocations. Hence: if the parent is - * threaded these flags may not be used. These flags cannot be used if the parent uses threads or the child - * uses malloc(). 💣 */ +/* 💣 Note that FORK_NEW_USERNS, FORK_NEW_MOUNTNS, or FORK_NEW_NETNS should not be called in threaded + * programs, because they cause us to use raw_clone() which does not synchronize the glibc malloc() locks, + * and thus will cause deadlocks if the parent uses threads and the child does memory allocations. Hence: if + * the parent is threaded these flags may not be used. These flags cannot be used if the parent uses threads + * or the child uses malloc(). 💣 */ typedef enum ForkFlags { FORK_RESET_SIGNALS = 1 << 0, /* Reset all signal handlers and signal mask */ FORK_CLOSE_ALL_FDS = 1 << 1, /* Close all open file descriptors in the child, except for 0,1,2 */ @@ -179,6 +179,7 @@ typedef enum ForkFlags { FORK_CLOEXEC_OFF = 1 << 16, /* In the child: turn off O_CLOEXEC on all fds in except_fds[] */ FORK_KEEP_NOTIFY_SOCKET = 1 << 17, /* Unless this specified, $NOTIFY_SOCKET will be unset. */ FORK_DETACH = 1 << 18, /* Double fork if needed to ensure PID1/subreaper is parent */ + FORK_NEW_NETNS = 1 << 19, /* Run child in its own network namespace 💣 DO NOT USE IN THREADED PROGRAMS! 💣 */ } ForkFlags; int safe_fork_full( From 3f53332d4e3681582569f401b7e299bebe7fa751 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 19 Jan 2024 11:34:17 +0900 Subject: [PATCH 05/14] namespace-util: introduce netns_acquire() Similar to userns_acquire(), but for network namespace. --- src/basic/namespace-util.c | 27 +++++++++++++++++++++++++++ src/basic/namespace-util.h | 1 + 2 files changed, 28 insertions(+) diff --git a/src/basic/namespace-util.c b/src/basic/namespace-util.c index 2ecce5fa61..d517263b36 100644 --- a/src/basic/namespace-util.c +++ b/src/basic/namespace-util.c @@ -255,6 +255,33 @@ int userns_acquire(const char *uid_map, const char *gid_map) { return TAKE_FD(userns_fd); } +int netns_acquire(void) { + _cleanup_(sigkill_waitp) pid_t pid = 0; + _cleanup_close_ int netns_fd = -EBADF; + int r; + + /* Forks off a process in a new network namespace, acquires a network namespace fd, and then kills + * the process again. This way we have a netns fd that is not bound to any process. */ + + r = safe_fork("(sd-mknetns)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGKILL|FORK_NEW_NETNS, &pid); + if (r < 0) + return log_debug_errno(r, "Failed to fork process (sd-mknetns): %m"); + if (r == 0) + /* Child. We do nothing here, just freeze until somebody kills us. */ + freeze(); + + r = namespace_open(pid, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, + &netns_fd, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); + if (r < 0) + return log_debug_errno(r, "Failed to open netns fd: %m"); + + return TAKE_FD(netns_fd); +} + int in_same_namespace(pid_t pid1, pid_t pid2, NamespaceType type) { const char *ns_path; struct stat ns_st1, ns_st2; diff --git a/src/basic/namespace-util.h b/src/basic/namespace-util.h index cd5b8ecab5..d1d015612f 100644 --- a/src/basic/namespace-util.h +++ b/src/basic/namespace-util.h @@ -51,4 +51,5 @@ static inline bool userns_shift_range_valid(uid_t shift, uid_t range) { } int userns_acquire(const char *uid_map, const char *gid_map); +int netns_acquire(void); int in_same_namespace(pid_t pid1, pid_t pid2, NamespaceType type); From a3df693799499a26735acc1f0c4f1b5d1f182fa7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 14:09:05 +0900 Subject: [PATCH 06/14] udev-util: introduce reset_cached_udev_availability() --- src/shared/udev-util.c | 15 ++++++++++----- src/shared/udev-util.h | 1 + 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index 7a86289d16..4f4b675a37 100644 --- a/src/shared/udev-util.c +++ b/src/shared/udev-util.c @@ -14,6 +14,7 @@ #include "id128-util.h" #include "log.h" #include "macro.h" +#include "missing_threads.h" #include "parse-util.h" #include "path-util.h" #include "signal-util.h" @@ -363,18 +364,22 @@ int udev_queue_is_empty(void) { (errno == ENOENT ? true : -errno) : false; } -bool udev_available(void) { - static int cache = -1; +static int cached_udev_availability = -1; +void reset_cached_udev_availability(void) { + cached_udev_availability = -1; +} + +bool udev_available(void) { /* The service systemd-udevd is started only when /sys is read write. * See systemd-udevd.service: ConditionPathIsReadWrite=/sys * Also, our container interface (http://systemd.io/CONTAINER_INTERFACE/) states that /sys must * be mounted in read-only mode in containers. */ - if (cache >= 0) - return cache; + if (cached_udev_availability >= 0) + return cached_udev_availability; - return (cache = (path_is_read_only_fs("/sys/") <= 0)); + return (cached_udev_availability = (path_is_read_only_fs("/sys/") <= 0)); } int device_get_vendor_string(sd_device *device, const char **ret) { diff --git a/src/shared/udev-util.h b/src/shared/udev-util.h index 5f49e87116..4d27bed1d9 100644 --- a/src/shared/udev-util.h +++ b/src/shared/udev-util.h @@ -23,6 +23,7 @@ size_t udev_replace_chars(char *str, const char *allow); int udev_queue_is_empty(void); +void reset_cached_udev_availability(void); bool udev_available(void); int device_get_vendor_string(sd_device *device, const char **ret); From 3e98379ec06804a532e7bdf23ec9439b7be35693 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Jan 2024 16:07:58 +0900 Subject: [PATCH 07/14] sd-device: introduce device_get_sysattr_u32() --- src/libsystemd/sd-device/device-private.h | 1 + src/libsystemd/sd-device/sd-device.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 79a0013dc7..e1f3b6e80c 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -21,6 +21,7 @@ int device_get_property_bool(sd_device *device, const char *key); int device_get_property_int(sd_device *device, const char *key, int *ret); int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value); int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value); +int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret_value); int device_get_sysattr_bool(sd_device *device, const char *sysattr); int device_get_device_id(sd_device *device, const char **ret); int device_get_devlink_priority(sd_device *device, int *ret); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index f18c8e6922..22b6437823 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2423,6 +2423,25 @@ int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned return v > 0; } +int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret_value) { + const char *value; + int r; + + r = sd_device_get_sysattr_value(device, sysattr, &value); + if (r < 0) + return r; + + uint32_t v; + r = safe_atou32(value, &v); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); + + if (ret_value) + *ret_value = v; + /* We return "true" if the value is positive. */ + return v > 0; +} + int device_get_sysattr_bool(sd_device *device, const char *sysattr) { const char *value; int r; From 6e931bc578b9221edf5e48c5a7673569d2ed9c01 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 19 Jan 2024 12:35:25 +0900 Subject: [PATCH 08/14] sd-netlink: introduce rtnl_rename_link() --- src/libsystemd/sd-netlink/netlink-util.c | 38 ++++++++++++++++++++++++ src/libsystemd/sd-netlink/netlink-util.h | 1 + src/libsystemd/sd-netlink/test-netlink.c | 13 ++++++++ 3 files changed, 52 insertions(+) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 832159a649..a591c7d6d6 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -20,6 +20,13 @@ static int set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { assert(name); /* Assign the requested name. */ + + if (!*rtnl) { + r = sd_netlink_open(rtnl); + if (r < 0) + return r; + } + r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); if (r < 0) return r; @@ -31,6 +38,37 @@ static int set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { return sd_netlink_call(*rtnl, message, 0, NULL); } +int rtnl_rename_link(sd_netlink **rtnl, const char *orig_name, const char *new_name) { + _cleanup_(sd_netlink_unrefp) sd_netlink *our_rtnl = NULL; + int r, ifindex; + + assert(orig_name); + assert(new_name); + + /* This does not check alternative names. Callers must check the requested name is not used as an + * alternative name. */ + + if (streq(orig_name, new_name)) + return 0; + + if (!ifname_valid(new_name)) + return -EINVAL; + + if (!rtnl) + rtnl = &our_rtnl; + if (!*rtnl) { + r = sd_netlink_open(rtnl); + if (r < 0) + return r; + } + + ifindex = rtnl_resolve_ifname(rtnl, orig_name); + if (ifindex < 0) + return ifindex; + + return set_link_name(rtnl, ifindex, new_name); +} + int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name, char* const *alternative_names) { _cleanup_strv_free_ char **original_altnames = NULL, **new_altnames = NULL; bool altname_deleted = false; diff --git a/src/libsystemd/sd-netlink/netlink-util.h b/src/libsystemd/sd-netlink/netlink-util.h index 56bbfd30d1..b895ac41d1 100644 --- a/src/libsystemd/sd-netlink/netlink-util.h +++ b/src/libsystemd/sd-netlink/netlink-util.h @@ -31,6 +31,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(MultipathRoute*, multipath_route_free); int multipath_route_dup(const MultipathRoute *m, MultipathRoute **ret); +int rtnl_rename_link(sd_netlink **rtnl, const char *orig_name, const char *new_name); int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name, char* const* alternative_names); static inline int rtnl_append_link_alternative_names(sd_netlink **rtnl, int ifindex, char* const *alternative_names) { return rtnl_set_link_name(rtnl, ifindex, NULL, alternative_names); diff --git a/src/libsystemd/sd-netlink/test-netlink.c b/src/libsystemd/sd-netlink/test-netlink.c index 13aedc4dbe..fb3150f131 100644 --- a/src/libsystemd/sd-netlink/test-netlink.c +++ b/src/libsystemd/sd-netlink/test-netlink.c @@ -681,6 +681,19 @@ TEST(rtnl_set_link_name) { _cleanup_free_ char *resolved = NULL; assert_se(rtnl_resolve_link_alternative_name(&rtnl, "test-additional-name", &resolved) == ifindex); assert_se(streq_ptr(resolved, "test-shortname")); + resolved = mfree(resolved); + + assert_se(rtnl_rename_link(&rtnl, "test-shortname", "test-shortname") >= 0); + assert_se(rtnl_rename_link(&rtnl, "test-shortname", "test-shortname2") >= 0); + assert_se(rtnl_rename_link(NULL, "test-shortname2", "test-shortname3") >= 0); + + assert_se(rtnl_resolve_link_alternative_name(&rtnl, "test-additional-name", &resolved) == ifindex); + assert_se(streq_ptr(resolved, "test-shortname3")); + resolved = mfree(resolved); + + assert_se(rtnl_resolve_link_alternative_name(&rtnl, "test-shortname3", &resolved) == ifindex); + assert_se(streq_ptr(resolved, "test-shortname3")); + resolved = mfree(resolved); } DEFINE_TEST_MAIN(LOG_DEBUG); From d92686407398cc48a35511e0e74f4e9b06624c3a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 10:32:10 +0900 Subject: [PATCH 09/14] sd-netlink: add policy for NL80211_ATTR_NETNS_FD --- src/libsystemd/sd-netlink/netlink-types-genl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsystemd/sd-netlink/netlink-types-genl.c b/src/libsystemd/sd-netlink/netlink-types-genl.c index 6fe9adcdbd..226ac865c9 100644 --- a/src/libsystemd/sd-netlink/netlink-types-genl.c +++ b/src/libsystemd/sd-netlink/netlink-types-genl.c @@ -199,6 +199,7 @@ static const NLAPolicy genl_nl80211_policies[] = { [NL80211_ATTR_SSID] = BUILD_POLICY_WITH_SIZE(BINARY, IEEE80211_MAX_SSID_LEN), [NL80211_ATTR_STATUS_CODE] = BUILD_POLICY(U16), [NL80211_ATTR_4ADDR] = BUILD_POLICY(U8), + [NL80211_ATTR_NETNS_FD] = BUILD_POLICY(U32), }; /***************** genl wireguard type systems *****************/ From cdd9988e430007bba73bb3b57dddd9cf1ef05975 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 09:28:31 +0900 Subject: [PATCH 10/14] nspawn-network: split out move_back_network_interfaces() No functional change, just refactoring and preparation for later commits. --- src/nspawn/nspawn-network.c | 79 +++++++++++++++++++++++++++++++++++++ src/nspawn/nspawn-network.h | 1 + src/nspawn/nspawn.c | 39 +----------------- 3 files changed, 81 insertions(+), 38 deletions(-) diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 369d8742b6..de21c68a6d 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -11,13 +11,16 @@ #include "alloc-util.h" #include "ether-addr-util.h" +#include "fd-util.h" #include "hexdecoct.h" #include "lock-util.h" #include "missing_network.h" +#include "namespace-util.h" #include "netif-naming-scheme.h" #include "netlink-util.h" #include "nspawn-network.h" #include "parse-util.h" +#include "process-util.h" #include "siphash24.h" #include "socket-netlink.h" #include "socket-util.h" @@ -503,6 +506,58 @@ int test_network_interfaces_initialized(char **iface_pairs) { return 0; } +static int netns_child_begin(int netns_fd, int *ret_original_netns_fd) { + _cleanup_close_ int original_netns_fd = -EBADF; + int r; + + assert(netns_fd >= 0); + + if (ret_original_netns_fd) { + r = namespace_open(0, + /* ret_pidns_fd = */ NULL, + /* ret_mntns_fd = */ NULL, + &original_netns_fd, + /* ret_userns_fd = */ NULL, + /* ret_root_fd = */ NULL); + if (r < 0) + return log_error_errno(r, "Failed to open original network namespace: %m"); + } + + r = namespace_enter(/* pidns_fd = */ -EBADF, + /* mntns_fd = */ -EBADF, + netns_fd, + /* userns_fd = */ -EBADF, + /* root_fd = */ -EBADF); + if (r < 0) + return log_error_errno(r, "Failed to enter child network namespace: %m"); + + if (ret_original_netns_fd) + *ret_original_netns_fd = TAKE_FD(original_netns_fd); + + return 0; +} + +static int netns_fork_and_wait(int netns_fd, int *ret_original_netns_fd) { + int r; + + assert(netns_fd >= 0); + + r = safe_fork("(sd-netns)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_WAIT|FORK_LOG, NULL); + if (r < 0) + return log_error_errno(r, "Failed to fork process (sd-netns): %m"); + if (r == 0) { + if (netns_child_begin(netns_fd, ret_original_netns_fd) < 0) + _exit(EXIT_FAILURE); + + return 0; + } + + if (ret_original_netns_fd) + *ret_original_netns_fd = -EBADF; + + return 1; +} + int move_network_interfaces(int netns_fd, char **iface_pairs) { _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL; int r; @@ -544,6 +599,30 @@ int move_network_interfaces(int netns_fd, char **iface_pairs) { return 0; } +int move_back_network_interfaces(int child_netns_fd, char **interface_pairs) { + _cleanup_close_ int parent_netns_fd = -EBADF; + int r; + + assert(child_netns_fd >= 0); + + if (strv_isempty(interface_pairs)) + return 0; + + r = netns_fork_and_wait(child_netns_fd, &parent_netns_fd); + if (r < 0) + return r; + if (r == 0) { + /* Reverse network interfaces pair list so that interfaces get their initial name back. + * This is about ensuring interfaces get their old name back when being moved back. */ + interface_pairs = strv_reverse(interface_pairs); + + r = move_network_interfaces(parent_netns_fd, interface_pairs); + _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); + } + + return 0; +} + int setup_macvlan(const char *machine_name, pid_t pid, char **iface_pairs) { _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL; unsigned idx = 0; diff --git a/src/nspawn/nspawn-network.h b/src/nspawn/nspawn-network.h index a785f8ea42..9729b6e37e 100644 --- a/src/nspawn/nspawn-network.h +++ b/src/nspawn/nspawn-network.h @@ -19,6 +19,7 @@ int setup_macvlan(const char *machine_name, pid_t pid, char **iface_pairs); int setup_ipvlan(const char *machine_name, pid_t pid, char **iface_pairs); int move_network_interfaces(int netns_fd, char **iface_pairs); +int move_back_network_interfaces(int child_netns_fd, char **interface_pairs); int veth_extra_parse(char ***l, const char *p); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 5dd3ff5549..445d85da6f 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5377,46 +5377,9 @@ static int run_container( fd_kmsg_fifo = safe_close(fd_kmsg_fifo); if (arg_private_network) { - /* Move network interfaces back to the parent network namespace. We use `safe_fork` - * to avoid having to move the parent to the child network namespace. */ - r = safe_fork(NULL, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_WAIT|FORK_LOG, NULL); + r = move_back_network_interfaces(child_netns_fd, arg_network_interfaces); if (r < 0) return r; - - if (r == 0) { - _cleanup_close_ int parent_netns_fd = -EBADF; - - r = namespace_open(0, - /* ret_pidns_fd = */ NULL, - /* ret_mntns_fd = */ NULL, - &parent_netns_fd, - /* ret_userns_fd = */ NULL, - /* ret_root_fd = */ NULL); - if (r < 0) { - log_error_errno(r, "Failed to open parent network namespace: %m"); - _exit(EXIT_FAILURE); - } - - r = namespace_enter(/* pidns_fd = */ -EBADF, - /* mntns_fd = */ -EBADF, - child_netns_fd, - /* userns_fd = */ -EBADF, - /* root_fd = */ -EBADF); - if (r < 0) { - log_error_errno(r, "Failed to enter child network namespace: %m"); - _exit(EXIT_FAILURE); - } - - /* Reverse network interfaces pair list so that interfaces get their initial name back. - * This is about ensuring interfaces get their old name back when being moved back. */ - arg_network_interfaces = strv_reverse(arg_network_interfaces); - - r = move_network_interfaces(parent_netns_fd, arg_network_interfaces); - if (r < 0) - log_error_errno(r, "Failed to move network interfaces back to parent network namespace: %m"); - - _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); - } } r = wait_for_container(TAKE_PID(*pid), &container_status); From 38475cac6f7c3d74b3c0a4468d9a61ae76a127ad Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 09:48:12 +0900 Subject: [PATCH 11/14] nspawn-network: split out move_network_interface_one() This also changes to use sd_device to get some attributes. So, on moving interfaces back to the parent, we need to populate sysfs associated to the client netns. That may look redundant and complicated, but it makes later change easier, and hopefully faster. --- src/nspawn/nspawn-network.c | 101 ++++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 26 deletions(-) diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index de21c68a6d..27ae07eba4 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -4,17 +4,21 @@ #include #include #include +#include #include "sd-device.h" #include "sd-id128.h" #include "sd-netlink.h" #include "alloc-util.h" +#include "device-util.h" #include "ether-addr-util.h" #include "fd-util.h" #include "hexdecoct.h" #include "lock-util.h" #include "missing_network.h" +#include "mkdir.h" +#include "mount-util.h" #include "namespace-util.h" #include "netif-naming-scheme.h" #include "netlink-util.h" @@ -531,6 +535,22 @@ static int netns_child_begin(int netns_fd, int *ret_original_netns_fd) { if (r < 0) return log_error_errno(r, "Failed to enter child network namespace: %m"); + r = umount_recursive("/sys/", /* flags = */ 0); + if (r < 0) + log_debug_errno(r, "Failed to unmount directories below /sys/, ignoring: %m"); + + (void) mkdir_p("/sys/", 0755); + + /* Populate new sysfs instance associated with the client netns, to make sd_device usable. */ + r = mount_nofollow_verbose(LOG_ERR, "sysfs", "/sys/", "sysfs", + MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, /* opts = */ NULL); + if (r < 0) + return log_error_errno(r, "Failed to mount sysfs on /sys/: %m"); + + /* udev_avaliable() might be called previously and the result may be cached. + * Now, we (re-)mount sysfs. Hence, we need to reset the cache. */ + reset_cached_udev_availability(); + if (ret_original_netns_fd) *ret_original_netns_fd = TAKE_FD(original_netns_fd); @@ -542,7 +562,7 @@ static int netns_fork_and_wait(int netns_fd, int *ret_original_netns_fd) { assert(netns_fd >= 0); - r = safe_fork("(sd-netns)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_WAIT|FORK_LOG, NULL); + r = safe_fork("(sd-netns)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_WAIT|FORK_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE, NULL); if (r < 0) return log_error_errno(r, "Failed to fork process (sd-netns): %m"); if (r == 0) { @@ -558,42 +578,71 @@ static int netns_fork_and_wait(int netns_fd, int *ret_original_netns_fd) { return 1; } +static int move_network_interface_one(sd_netlink **rtnl, int netns_fd, sd_device *dev, const char *name) { + _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; + int r; + + assert(rtnl); + assert(netns_fd >= 0); + assert(dev); + assert(name); + + if (!*rtnl) { + r = sd_netlink_open(rtnl); + if (r < 0) + return log_error_errno(r, "Failed to connect to rtnetlink: %m"); + } + + int ifindex; + r = sd_device_get_ifindex(dev, &ifindex); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to get ifindex: %m"); + + r = sd_rtnl_message_new_link(*rtnl, &m, RTM_SETLINK, ifindex); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to allocate netlink message: %m"); + + r = sd_netlink_message_append_u32(m, IFLA_NET_NS_FD, netns_fd); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to append namespace fd to netlink message: %m"); + + const char *sysname; + r = sd_device_get_sysname(dev, &sysname); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to get sysname: %m"); + + if (!streq(name, sysname)) { + r = sd_netlink_message_append_string(m, IFLA_IFNAME, name); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to add netlink interface name: %m"); + } + + r = sd_netlink_call(*rtnl, m, 0, NULL); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to move interface to namespace: %m"); + + return 0; +} + int move_network_interfaces(int netns_fd, char **iface_pairs) { _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL; int r; + assert(netns_fd >= 0); + if (strv_isempty(iface_pairs)) return 0; - r = sd_netlink_open(&rtnl); - if (r < 0) - return log_error_errno(r, "Failed to connect to netlink: %m"); + STRV_FOREACH_PAIR(from, to, iface_pairs) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - STRV_FOREACH_PAIR(i, b, iface_pairs) { - _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; - int ifi; - - ifi = rtnl_resolve_interface_or_warn(&rtnl, *i); - if (ifi < 0) - return ifi; - - r = sd_rtnl_message_new_link(rtnl, &m, RTM_SETLINK, ifi); + r = sd_device_new_from_ifname(&dev, *from); if (r < 0) - return log_error_errno(r, "Failed to allocate netlink message: %m"); + return log_error_errno(r, "Unknown interface name %s: %m", *from); - r = sd_netlink_message_append_u32(m, IFLA_NET_NS_FD, netns_fd); + r = move_network_interface_one(&rtnl, netns_fd, dev, *to); if (r < 0) - return log_error_errno(r, "Failed to append namespace fd to netlink message: %m"); - - if (!streq(*b, *i)) { - r = sd_netlink_message_append_string(m, IFLA_IFNAME, *b); - if (r < 0) - return log_error_errno(r, "Failed to add netlink interface name: %m"); - } - - r = sd_netlink_call(rtnl, m, 0, NULL); - if (r < 0) - return log_error_errno(r, "Failed to move interface %s to namespace: %m", *i); + return r; } return 0; From 326214bfa8f3efb6886306df3cdcd85a68830448 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 10:07:19 +0900 Subject: [PATCH 12/14] nspawn-network: also check alternative names If the requested new name for a network interface is already assigned as a alternative name, then it is not necessary to and cannot rename the interface. --- src/nspawn/nspawn-network.c | 39 ++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 27ae07eba4..a05a2bc9e0 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -578,6 +578,37 @@ static int netns_fork_and_wait(int netns_fd, int *ret_original_netns_fd) { return 1; } +static int needs_rename(sd_netlink **rtnl, sd_device *dev, const char *name) { + int r; + + assert(rtnl); + assert(dev); + assert(name); + + const char *ifname; + r = sd_device_get_sysname(dev, &ifname); + if (r < 0) + return r; + + if (streq(name, ifname)) + return false; + + int ifindex; + r = sd_device_get_ifindex(dev, &ifindex); + if (r < 0) + return r; + + _cleanup_strv_free_ char **altnames = NULL; + r = rtnl_get_link_alternative_names(rtnl, ifindex, &altnames); + if (r == -EOPNOTSUPP) + return true; /* alternative interface name is not supported, hence the name is not + * assigned to the interface. */ + if (r < 0) + return r; + + return !strv_contains(altnames, name); +} + static int move_network_interface_one(sd_netlink **rtnl, int netns_fd, sd_device *dev, const char *name) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int r; @@ -606,12 +637,10 @@ static int move_network_interface_one(sd_netlink **rtnl, int netns_fd, sd_device if (r < 0) return log_device_error_errno(dev, r, "Failed to append namespace fd to netlink message: %m"); - const char *sysname; - r = sd_device_get_sysname(dev, &sysname); + r = needs_rename(rtnl, dev, name); if (r < 0) - return log_device_error_errno(dev, r, "Failed to get sysname: %m"); - - if (!streq(name, sysname)) { + return log_device_error_errno(dev, r, "Failed to determine if the interface should be renamed to '%s': %m", name); + if (r > 0) { r = sd_netlink_message_append_string(m, IFLA_IFNAME, name); if (r < 0) return log_device_error_errno(dev, r, "Failed to add netlink interface name: %m"); From 1c3e5b42103c5edba7162a1bac9f8541d338ea4f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 10:36:28 +0900 Subject: [PATCH 13/14] nspawn-network: support passing wireless interface to container Closes #7873. --- src/nspawn/nspawn-network.c | 120 +++++++++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 2 deletions(-) diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index a05a2bc9e0..151bb1d69a 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -11,6 +12,7 @@ #include "sd-netlink.h" #include "alloc-util.h" +#include "device-private.h" #include "device-util.h" #include "ether-addr-util.h" #include "fd-util.h" @@ -609,6 +611,116 @@ static int needs_rename(sd_netlink **rtnl, sd_device *dev, const char *name) { return !strv_contains(altnames, name); } +static int move_wlan_interface_impl(sd_netlink **genl, int netns_fd, sd_device *dev) { + _cleanup_(sd_netlink_unrefp) sd_netlink *our_genl = NULL; + _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; + int r; + + assert(netns_fd >= 0); + assert(dev); + + if (!genl) + genl = &our_genl; + if (!*genl) { + r = sd_genl_socket_open(genl); + if (r < 0) + return log_error_errno(r, "Failed to connect to generic netlink: %m"); + } + + r = sd_genl_message_new(*genl, NL80211_GENL_NAME, NL80211_CMD_SET_WIPHY_NETNS, &m); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to allocate netlink message: %m"); + + uint32_t phy_index; + r = device_get_sysattr_u32(dev, "phy80211/index", &phy_index); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to get phy index: %m"); + + r = sd_netlink_message_append_u32(m, NL80211_ATTR_WIPHY, phy_index); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to append phy index to netlink message: %m"); + + r = sd_netlink_message_append_u32(m, NL80211_ATTR_NETNS_FD, netns_fd); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to append namespace fd to netlink message: %m"); + + r = sd_netlink_call(*genl, m, 0, NULL); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to move interface to namespace: %m"); + + return 0; +} + +static int move_wlan_interface_one( + sd_netlink **rtnl, + sd_netlink **genl, + int *temp_netns_fd, + int netns_fd, + sd_device *dev, + const char *name) { + + int r; + + assert(rtnl); + assert(genl); + assert(temp_netns_fd); + assert(netns_fd >= 0); + assert(dev); + assert(name); + + r = needs_rename(rtnl, dev, name); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to determine if the interface should be renamed to '%s': %m", name); + if (r == 0) + return move_wlan_interface_impl(genl, netns_fd, dev); + + /* The command NL80211_CMD_SET_WIPHY_NETNS takes phy instead of network interface, and does not take + * an interface name in the passed network namespace. Hence, we need to move the phy and interface to + * a temporary network namespace, rename the interface in it, and move them to the requested netns. */ + + if (*temp_netns_fd < 0) { + r = netns_acquire(); + if (r < 0) + return log_error_errno(r, "Failed to acquire new network namespace: %m"); + *temp_netns_fd = r; + } + + r = move_wlan_interface_impl(genl, *temp_netns_fd, dev); + if (r < 0) + return r; + + const char *sysname; + r = sd_device_get_sysname(dev, &sysname); + if (r < 0) + return log_device_error_errno(dev, r, "Failed to get interface name: %m"); + + r = netns_fork_and_wait(*temp_netns_fd, NULL); + if (r < 0) + return log_error_errno(r, "Failed to fork process (nspawn-rename-wlan): %m"); + if (r == 0) { + _cleanup_(sd_device_unrefp) sd_device *temp_dev = NULL; + + r = rtnl_rename_link(NULL, sysname, name); + if (r < 0) { + log_error_errno(r, "Failed to rename network interface '%s' to '%s': %m", sysname, name); + goto finalize; + } + + r = sd_device_new_from_ifname(&temp_dev, name); + if (r < 0) { + log_error_errno(r, "Failed to acquire device '%s': %m", name); + goto finalize; + } + + r = move_wlan_interface_impl(NULL, netns_fd, temp_dev); + + finalize: + _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); + } + + return 0; +} + static int move_network_interface_one(sd_netlink **rtnl, int netns_fd, sd_device *dev, const char *name) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int r; @@ -654,7 +766,8 @@ static int move_network_interface_one(sd_netlink **rtnl, int netns_fd, sd_device } int move_network_interfaces(int netns_fd, char **iface_pairs) { - _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL; + _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL, *genl = NULL; + _cleanup_close_ int temp_netns_fd = -EBADF; int r; assert(netns_fd >= 0); @@ -669,7 +782,10 @@ int move_network_interfaces(int netns_fd, char **iface_pairs) { if (r < 0) return log_error_errno(r, "Unknown interface name %s: %m", *from); - r = move_network_interface_one(&rtnl, netns_fd, dev, *to); + if (device_is_devtype(dev, "wlan")) + r = move_wlan_interface_one(&rtnl, &genl, &temp_netns_fd, netns_fd, dev, *to); + else + r = move_network_interface_one(&rtnl, netns_fd, dev, *to); if (r < 0) return r; } From e8736fb6c93815b3e3b5ac7415cbc67e731dfb99 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Jan 2024 10:55:35 +0900 Subject: [PATCH 14/14] test-13-NSPAWN: add more test case for passing network interfaces - test interface renaming that conflicts with the current alternative network interface name, - test passing wlan interfaces. --- test/TEST-13-NSPAWN/test.sh | 4 ++++ test/units/testsuite-13.nspawn.sh | 22 +++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/test/TEST-13-NSPAWN/test.sh b/test/TEST-13-NSPAWN/test.sh index 2e94156432..28d845c94b 100755 --- a/test/TEST-13-NSPAWN/test.sh +++ b/test/TEST-13-NSPAWN/test.sh @@ -13,6 +13,10 @@ test_append_files() { local workspace="${1:?}" local container="$workspace/testsuite-13-container-template" + # For virtual wlan interface. + instmods mac80211_hwsim + generate_module_dependencies + # Create a dummy container "template" with a minimal toolset, which we can # then use as a base for our nspawn/machinectl tests initdir="$container" setup_basic_dirs diff --git a/test/units/testsuite-13.nspawn.sh b/test/units/testsuite-13.nspawn.sh index 8a6fa84dc9..e984d585ca 100755 --- a/test/units/testsuite-13.nspawn.sh +++ b/test/units/testsuite-13.nspawn.sh @@ -324,7 +324,7 @@ EOF } nspawn_settings_cleanup() { - for dev in sd-host-only sd-shared{1,2} sd-macvlan{1,2} sd-ipvlan{1,2}; do + for dev in sd-host-only sd-shared{1,2,3} sd-macvlan{1,2} sd-ipvlan{1,2}; do ip link del "$dev" || : done @@ -332,7 +332,7 @@ nspawn_settings_cleanup() { } testcase_nspawn_settings() { - local root container dev private_users + local root container dev private_users wlan_names='' wlan_checks='' mkdir -p /run/systemd/nspawn root="$(mktemp -d /var/lib/machines/testsuite-13.nspawn-settings.XXX)" @@ -341,10 +341,17 @@ testcase_nspawn_settings() { rm -f "/etc/systemd/nspawn/$container.nspawn" mkdir -p "$root/tmp" "$root"/opt/{tmp,inaccessible,also-inaccessible} - for dev in sd-host-only sd-shared{1,2} sd-macvlan{1,2} sd-macvlanloong sd-ipvlan{1,2} sd-ipvlanlooong; do + # add virtual wlan interfaces + if modprobe mac80211_hwsim radios=2; then + wlan_names='wlan0 wlan1:wl-renamed1' + wlan_checks='ip link | grep wlan0\nip link | grep wl-renamed1' + fi + + for dev in sd-host-only sd-shared{1,2,3} sd-macvlan{1,2} sd-macvlanloong sd-ipvlan{1,2} sd-ipvlanlooong; do ip link add "$dev" type dummy done udevadm settle + ip link property add dev sd-shared3 altname sd-altname3 altname sd-altname-tooooooooooooo-long ip link trap nspawn_settings_cleanup RETURN @@ -394,7 +401,7 @@ Private=yes VirtualEthernet=yes VirtualEthernetExtra=my-fancy-veth1 VirtualEthernetExtra=fancy-veth2:my-fancy-veth2 -Interface=sd-shared1 sd-shared2:sd-shared2 +Interface=sd-shared1 sd-shared2:sd-renamed2 sd-shared3:sd-altname3 ${wlan_names} MACVLAN=sd-macvlan1 sd-macvlan2:my-macvlan2 sd-macvlanloong IPVLAN=sd-ipvlan1 sd-ipvlan2:my-ipvlan2 sd-ipvlanlooong Zone=sd-zone0 @@ -437,12 +444,17 @@ ip link | grep host0@ ip link | grep my-fancy-veth1@ ip link | grep my-fancy-veth2@ ip link | grep sd-shared1 -ip link | grep sd-shared2 +ip link | grep sd-renamed2 +ip link | grep sd-shared3 +ip link | grep sd-altname3 +ip link | grep sd-altname-tooooooooooooo-long ip link | grep mv-sd-macvlan1@ ip link | grep my-macvlan2@ ip link | grep iv-sd-ipvlan1@ ip link | grep my-ipvlan2@ EOF + echo -e "$wlan_checks" >>"$root/entrypoint.sh" + timeout 30 systemd-nspawn --directory="$root" # And now for stuff that needs to run separately