Merge pull request #33809 from YHNdnzj/pidref-namespace

namespace-util: introduce pidref_namespace_open() and use it where appropriate; clean up mount-util a bit along the way

5 files changed, 134 insertions, 83 deletions

diff --git a/src/basic/namespace-util.c b/src/basic/namespace-util.c
index 5b4e43f921..d9ad25fdd7 100644
--- a/src/basic/namespace-util.c
+++ b/src/basic/namespace-util.c
@@ -17,7 +17,7 @@
 #include "stdio-util.h"
 #include "user-util.h"
 
-const struct namespace_info namespace_info[] = {
+const struct namespace_info namespace_info[_NAMESPACE_TYPE_MAX + 1] = {
         [NAMESPACE_CGROUP] =  { "cgroup", "ns/cgroup", CLONE_NEWCGROUP,                          },
         [NAMESPACE_IPC]    =  { "ipc",    "ns/ipc",    CLONE_NEWIPC,                             },
         [NAMESPACE_NET]    =  { "net",    "ns/net",    CLONE_NEWNET,                             },
@@ -42,8 +42,8 @@ static NamespaceType clone_flag_to_namespace_type(unsigned long clone_flag) {
         return _NAMESPACE_TYPE_INVALID;
 }
 
-int namespace_open(
-                pid_t pid,
+int pidref_namespace_open(
+                const PidRef *pidref,
                 int *ret_pidns_fd,
                 int *ret_mntns_fd,
                 int *ret_netns_fd,
@@ -52,13 +52,14 @@ int namespace_open(
 
         _cleanup_close_ int pidns_fd = -EBADF, mntns_fd = -EBADF, netns_fd = -EBADF,
                 userns_fd = -EBADF, root_fd = -EBADF;
+        int r;
 
-        assert(pid >= 0);
+        assert(pidref_is_set(pidref));
 
         if (ret_pidns_fd) {
                 const char *pidns;
 
-                pidns = pid_namespace_path(pid, NAMESPACE_PID);
+                pidns = pid_namespace_path(pidref->pid, NAMESPACE_PID);
                 pidns_fd = open(pidns, O_RDONLY|O_NOCTTY|O_CLOEXEC);
                 if (pidns_fd < 0)
                         return -errno;
@@ -67,7 +68,7 @@ int namespace_open(
         if (ret_mntns_fd) {
                 const char *mntns;
 
-                mntns = pid_namespace_path(pid, NAMESPACE_MOUNT);
+                mntns = pid_namespace_path(pidref->pid, NAMESPACE_MOUNT);
                 mntns_fd = open(mntns, O_RDONLY|O_NOCTTY|O_CLOEXEC);
                 if (mntns_fd < 0)
                         return -errno;
@@ -76,7 +77,7 @@ int namespace_open(
         if (ret_netns_fd) {
                 const char *netns;
 
-                netns = pid_namespace_path(pid, NAMESPACE_NET);
+                netns = pid_namespace_path(pidref->pid, NAMESPACE_NET);
                 netns_fd = open(netns, O_RDONLY|O_NOCTTY|O_CLOEXEC);
                 if (netns_fd < 0)
                         return -errno;
@@ -85,7 +86,7 @@ int namespace_open(
         if (ret_userns_fd) {
                 const char *userns;
 
-                userns = pid_namespace_path(pid, NAMESPACE_USER);
+                userns = pid_namespace_path(pidref->pid, NAMESPACE_USER);
                 userns_fd = open(userns, O_RDONLY|O_NOCTTY|O_CLOEXEC);
                 if (userns_fd < 0 && errno != ENOENT)
                         return -errno;
@@ -94,12 +95,16 @@ int namespace_open(
         if (ret_root_fd) {
                 const char *root;
 
-                root = procfs_file_alloca(pid, "root");
+                root = procfs_file_alloca(pidref->pid, "root");
                 root_fd = open(root, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
                 if (root_fd < 0)
                         return -errno;
         }
 
+        r = pidref_verify(pidref);
+        if (r < 0)
+                return r;
+
         if (ret_pidns_fd)
                 *ret_pidns_fd = TAKE_FD(pidns_fd);
 
@@ -118,6 +123,22 @@ int namespace_open(
         return 0;
 }
 
+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) {
+
+        assert(pid >= 0);
+
+        if (pid == 0)
+                pid = getpid_cached();
+
+        return pidref_namespace_open(&PIDREF_MAKE_FROM_PID(pid), ret_pidns_fd, ret_mntns_fd, ret_netns_fd, ret_userns_fd, ret_root_fd);
+}
+
 int namespace_enter(int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd) {
         int r;
 
diff --git a/src/basic/namespace-util.h b/src/basic/namespace-util.h
index 545952a5d1..0a7616443d 100644
--- a/src/basic/namespace-util.h
+++ b/src/basic/namespace-util.h
@@ -3,6 +3,8 @@
 
 #include <sys/types.h>
 
+#include "pidref.h"
+
 typedef enum NamespaceType {
         NAMESPACE_CGROUP,
         NAMESPACE_IPC,
@@ -22,6 +24,13 @@ extern const struct namespace_info {
         unsigned int clone_flag;
 } namespace_info[_NAMESPACE_TYPE_MAX + 1];
 
+int pidref_namespace_open(
+                const PidRef *pidref,
+                int *ret_pidns_fd,
+                int *ret_mntns_fd,
+                int *ret_netns_fd,
+                int *ret_userns_fd,
+                int *ret_root_fd);
 int namespace_open(
                 pid_t pid,
                 int *ret_pidns_fd,
diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
index 86bd557b21..d4c6f1bfe3 100644
--- a/src/machine/machine-dbus.c
+++ b/src/machine/machine-dbus.c
@@ -232,12 +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,
-                                   /* ret_pidns_fd = */ NULL,
-                                   /* ret_mntns_fd = */ NULL,
-                                   &netns_fd,
-                                   /* ret_userns_fd = */ NULL,
-                                   /* ret_root_fd = */ NULL);
+                r = pidref_namespace_open(&m->leader,
+                                          /* ret_pidns_fd = */ NULL,
+                                          /* ret_mntns_fd = */ NULL,
+                                          &netns_fd,
+                                          /* ret_userns_fd = */ NULL,
+                                          /* ret_root_fd = */ NULL);
                 if (r < 0)
                         return r;
 
@@ -392,12 +392,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,
-                                   /* ret_netns_fd = */ NULL,
-                                   /* ret_userns_fd = */ NULL,
-                                   &root_fd);
+                r = pidref_namespace_open(&m->leader,
+                                          &pidns_fd,
+                                          &mntns_fd,
+                                          /* ret_netns_fd = */ NULL,
+                                          /* ret_userns_fd = */ NULL,
+                                          &root_fd);
                 if (r < 0)
                         return r;
 
@@ -1100,12 +1100,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,
-                                   /* ret_pidns_fd = */ NULL,
-                                   &mntns_fd,
-                                   /* ret_netns_fd = */ NULL,
-                                   /* ret_userns_fd = */ NULL,
-                                   &root_fd);
+                r = pidref_namespace_open(&m->leader,
+                                          /* 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/shared/mount-util.c b/src/shared/mount-util.c
index 7e18d12df4..aa0b9b40ec 100644
--- a/src/shared/mount-util.c
+++ b/src/shared/mount-util.c
@@ -759,27 +759,19 @@ int mount_exchange_graceful(int fsmount_fd, const char *dest, bool mount_beneath
          * this is not supported (minimum kernel v6.5), or if there is no mount on the mountpoint, we get
          * -EINVAL and then we fallback to normal mounting. */
 
-        r = RET_NERRNO(move_mount(
-                        fsmount_fd,
-                        /* from_path= */ "",
-                        /* to_fd= */ -EBADF,
-                        dest,
-                        MOVE_MOUNT_F_EMPTY_PATH | (mount_beneath ? MOVE_MOUNT_BENEATH : 0)));
+        r = RET_NERRNO(move_mount(fsmount_fd, /* from_path = */ "",
+                                  /* to_fd = */ -EBADF, dest,
+                                  MOVE_MOUNT_F_EMPTY_PATH | (mount_beneath ? MOVE_MOUNT_BENEATH : 0)));
         if (mount_beneath) {
+                if (r >= 0) /* Mounting beneath worked! Now unmount the upper mount. */
+                        return umount_verbose(LOG_DEBUG, dest, UMOUNT_NOFOLLOW|MNT_DETACH);
+
                 if (r == -EINVAL) { /* Fallback if mount_beneath is not supported */
                         log_debug_errno(r,
-                                        "Failed to mount beneath '%s', falling back to overmount",
+                                        "Cannot mount beneath '%s', falling back to overmount: %m",
                                         dest);
-                        return RET_NERRNO(move_mount(
-                                        fsmount_fd,
-                                        /* from_path= */ "",
-                                        /* to_fd= */ -EBADF,
-                                        dest,
-                                        MOVE_MOUNT_F_EMPTY_PATH));
+                        return mount_exchange_graceful(fsmount_fd, dest, /* mount_beneath = */ false);
                 }
-
-                if (r >= 0) /* If it is, now remove the old mount */
-                        return umount_verbose(LOG_DEBUG, dest, UMOUNT_NOFOLLOW|MNT_DETACH);
         }
 
         return r;
@@ -1091,52 +1083,44 @@ static int mount_in_namespace(
                 const char *dest,
                 bool read_only,
                 bool make_file_or_directory,
+                bool is_image,
                 const MountOptions *options,
-                const ImagePolicy *image_policy,
-                bool is_image) {
+                const ImagePolicy *image_policy) {
 
-        _cleanup_(dissected_image_unrefp) DissectedImage *img = NULL;
-        _cleanup_close_pair_ int errno_pipe_fd[2] = EBADF_PAIR;
-        _cleanup_close_ int mntns_fd = -EBADF, root_fd = -EBADF, pidns_fd = -EBADF, chased_src_fd = -EBADF,
-                            new_mount_fd = -EBADF;
+        _cleanup_close_ int mntns_fd = -EBADF, root_fd = -EBADF, pidns_fd = -EBADF, chased_src_fd = -EBADF;
         _cleanup_free_ char *chased_src_path = NULL;
         struct stat st;
-        pid_t child;
         int r;
 
         assert(propagate_path);
         assert(incoming_path);
         assert(src);
         assert(dest);
-        assert(!options || is_image);
+        assert(is_image || (!options && !image_policy));
 
         if (!pidref_is_set(target))
                 return -ESRCH;
 
-        r = namespace_open(target->pid, &pidns_fd, &mntns_fd, /* ret_netns_fd = */ NULL, /* ret_userns_fd = */ NULL, &root_fd);
+        r = pidref_namespace_open(target, &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");
 
-        r = in_same_namespace(target->pid, 0, NAMESPACE_MOUNT);
+        r = inode_same_at(mntns_fd, "", AT_FDCWD, "/proc/self/ns/mnt", AT_EMPTY_PATH);
         if (r < 0)
                 return log_debug_errno(r, "Failed to determine if mount namespaces are equal: %m");
         /* We can't add new mounts at runtime if the process wasn't started in a namespace */
         if (r > 0)
-                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to activate bind mount in target, not running in a mount namespace");
-
-        r = pidref_verify(target);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to verify target process '" PID_FMT "': %m", target->pid);
+                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to activate bind mount in target, not running in a mount namespace.");
 
         r = chase(src, NULL, 0, &chased_src_path, &chased_src_fd);
         if (r < 0)
-                return log_debug_errno(r, "Failed to resolve source path of %s: %m", src);
-        log_debug("Chased source path of %s to %s", src, chased_src_path);
+                return log_debug_errno(r, "Failed to resolve source path '%s': %m", src);
+        log_debug("Chased source path '%s': %s", src, chased_src_path);
 
         if (fstat(chased_src_fd, &st) < 0)
-                return log_debug_errno(errno, "Failed to stat() resolved source path %s: %m", src);
+                return log_debug_errno(errno, "Failed to stat() resolved source path '%s': %m", src);
         if (S_ISLNK(st.st_mode)) /* This shouldn't really happen, given that we just chased the symlinks above, but let's better be safe… */
-                return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Source directory %s can't be a symbolic link", src);
+                return log_debug_errno(SYNTHETIC_ERRNO(ELOOP), "Source path '%s' can't be a symbolic link.", src);
 
         if (!mount_new_api_supported()) /* Fallback if we can't use the new mount API */
                 return mount_in_namespace_legacy(
@@ -1155,6 +1139,11 @@ static int mount_in_namespace(
                                 image_policy,
                                 is_image);
 
+        _cleanup_(dissected_image_unrefp) DissectedImage *img = NULL;
+        _cleanup_close_ int new_mount_fd = -EBADF;
+        _cleanup_close_pair_ int errno_pipe_fd[2] = EBADF_PAIR;
+        pid_t child;
+
         if (is_image) {
                 r = verity_dissect_and_mount(
                                 chased_src_fd,
@@ -1170,10 +1159,9 @@ static int mount_in_namespace(
                                 /* verity= */ NULL,
                                 &img);
                 if (r < 0)
-                        return log_debug_errno(
-                                        r,
-                                        "Failed to dissect and mount image %s: %m",
-                                        chased_src_path);
+                        return log_debug_errno(r,
+                                               "Failed to dissect and mount image '%s': %m",
+                                               chased_src_path);
         } else {
                 new_mount_fd = open_tree(
                                 chased_src_fd,
@@ -1182,17 +1170,16 @@ static int mount_in_namespace(
                 if (new_mount_fd < 0)
                         return log_debug_errno(
                                         errno,
-                                        "Failed to open mount point \"%s\": %m",
+                                        "Failed to open mount source '%s': %m",
                                         chased_src_path);
 
                 if (read_only && mount_setattr(new_mount_fd, "", AT_EMPTY_PATH,
                                                &(struct mount_attr) {
                                                        .attr_set = MOUNT_ATTR_RDONLY,
                                                }, MOUNT_ATTR_SIZE_VER0) < 0)
-                        return log_debug_errno(
-                                        errno,
-                                        "Failed to set mount flags for \"%s\": %m",
-                                        chased_src_path);
+                        return log_debug_errno(errno,
+                                               "Failed to set mount for '%s' to read only: %m",
+                                               chased_src_path);
         }
 
         if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0)
@@ -1205,12 +1192,12 @@ static int mount_in_namespace(
                            FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM,
                            pidns_fd,
                            mntns_fd,
-                           /* netns_fd= */ -1,
-                           /* userns_fd= */ -1,
+                           /* netns_fd= */ -EBADF,
+                           /* userns_fd= */ -EBADF,
                            root_fd,
                            &child);
         if (r < 0)
-                return log_debug_errno(r, "Failed to fork off: %m");
+                return log_debug_errno(r, "Failed to fork off mount helper into namespace: %m");
         if (r == 0) {
                 errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]);
 
@@ -1258,7 +1245,7 @@ static int mount_in_namespace(
                 return log_debug_errno(r, "Failed to wait for child: %m");
         if (r != EXIT_SUCCESS) {
                 if (read(errno_pipe_fd[0], &r, sizeof(r)) == sizeof(r))
-                        return log_debug_errno(r, "Failed to mount: %m");
+                        return log_debug_errno(r, "Failed to mount into namespace: %m");
 
                 return log_debug_errno(SYNTHETIC_ERRNO(EPROTO), "Child failed.");
         }
@@ -1267,7 +1254,7 @@ static int mount_in_namespace(
 }
 
 int bind_mount_in_namespace(
-                PidRef * target,
+                const PidRef *target,
                 const char *propagate_path,
                 const char *incoming_path,
                 const char *src,
@@ -1275,11 +1262,20 @@ int bind_mount_in_namespace(
                 bool read_only,
                 bool make_file_or_directory) {
 
-        return mount_in_namespace(target, propagate_path, incoming_path, src, dest, read_only, make_file_or_directory, /* options= */ NULL, /* image_policy= */ NULL, /* is_image= */ false);
+        return mount_in_namespace(target,
+                                  propagate_path,
+                                  incoming_path,
+                                  src,
+                                  dest,
+                                  read_only,
+                                  make_file_or_directory,
+                                  /* is_image = */ false,
+                                  /* options = */ NULL,
+                                  /* image_policy = */ NULL);
 }
 
 int mount_image_in_namespace(
-                PidRef * target,
+                const PidRef *target,
                 const char *propagate_path,
                 const char *incoming_path,
                 const char *src,
@@ -1289,7 +1285,16 @@ int mount_image_in_namespace(
                 const MountOptions *options,
                 const ImagePolicy *image_policy) {
 
-        return mount_in_namespace(target, propagate_path, incoming_path, src, dest, read_only, make_file_or_directory, options, image_policy, /* is_image=*/ true);
+        return mount_in_namespace(target,
+                                  propagate_path,
+                                  incoming_path,
+                                  src,
+                                  dest,
+                                  read_only,
+                                  make_file_or_directory,
+                                  /* is_image = */ true,
+                                  options,
+                                  image_policy);
 }
 
 int make_mount_point(const char *path) {
diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h
index c260eef02a..8014eb48cf 100644
--- a/src/shared/mount-util.h
+++ b/src/shared/mount-util.h
@@ -70,8 +70,6 @@ int umount_verbose(
                 const char *where,
                 int flags);
 
-int mount_exchange_graceful(int fsmount_fd, const char *dest, bool mount_beneath);
-
 int mount_option_mangle(
                 const char *options,
                 unsigned long mount_flags,
@@ -106,8 +104,26 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(char*, umount_and_free);
 char* umount_and_unlink_and_free(char *p);
 DEFINE_TRIVIAL_CLEANUP_FUNC(char*, umount_and_unlink_and_free);
 
-int bind_mount_in_namespace(PidRef *target, const char *propagate_path, const char *incoming_path, const char *src, const char *dest, bool read_only, bool make_file_or_directory);
-int mount_image_in_namespace(PidRef *target, const char *propagate_path, const char *incoming_path, const char *src, const char *dest, bool read_only, bool make_file_or_directory, const MountOptions *options, const ImagePolicy *image_policy);
+int mount_exchange_graceful(int fsmount_fd, const char *dest, bool mount_beneath);
+
+int bind_mount_in_namespace(
+                const PidRef *target,
+                const char *propagate_path,
+                const char *incoming_path,
+                const char *src,
+                const char *dest,
+                bool read_only,
+                bool make_file_or_directory);
+int mount_image_in_namespace(
+                const PidRef *target,
+                const char *propagate_path,
+                const char *incoming_path,
+                const char *src,
+                const char *dest,
+                bool read_only,
+                bool make_file_or_directory,
+                const MountOptions *options,
+                const ImagePolicy *image_policy);
 
 int make_mount_point(const char *path);
 int fd_make_mount_point(int fd);