From 64341ce7b7b6b1eadbe65731d6074b8ac321f6c3 Mon Sep 17 00:00:00 2001 From: Alexander Mikhalitsyn Date: Fri, 26 Jan 2024 17:01:46 +0100 Subject: [PATCH 1/2] tree-wide: use container_uses_namespace() helper No functional changes. Will be useful in future support for an isolated user namespaces [1]. I have already played with that locally and found that in the LXC codebase we have a bunch of different ways to ensure if a container uses user namespaces or not. This commit contains a trivial conversion from an open-coded version of the container_uses_namespace() helper to an actual use of the helper. [1] https://lpc.events/event/17/contributions/1569/ Signed-off-by: Alexander Mikhalitsyn --- src/lxc/conf.c | 6 +++--- src/lxc/network.c | 2 +- src/lxc/start.c | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lxc/conf.c b/src/lxc/conf.c index a7775059df..c6c4279e2f 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -4290,7 +4290,7 @@ int lxc_sync_fds_parent(struct lxc_handler *handler) if (ret < 0) return syserror_ret(ret, "Failed to receive tty info from child process"); - if (handler->ns_clone_flags & CLONE_NEWNET) { + if (container_uses_namespace(handler, CLONE_NEWNET)) { ret = lxc_network_recv_name_and_ifindex_from_child(handler); if (ret < 0) return syserror_ret(ret, "Failed to receive names and ifindices for network devices from child"); @@ -4320,7 +4320,7 @@ int lxc_sync_fds_child(struct lxc_handler *handler) if (ret < 0) return syserror_ret(ret, "Failed to send tty file descriptors to parent"); - if (handler->ns_clone_flags & CLONE_NEWNET) { + if (container_uses_namespace(handler, CLONE_NEWNET)) { ret = lxc_network_send_name_and_ifindex_to_parent(handler); if (ret < 0) return syserror_ret(ret, "Failed to send network device names and ifindices to parent"); @@ -4382,7 +4382,7 @@ int lxc_setup(struct lxc_handler *handler) return log_error(-1, "Failed to setup container keyring"); } - if (handler->ns_clone_flags & CLONE_NEWNET) { + if (container_uses_namespace(handler, CLONE_NEWNET)) { ret = lxc_network_recv_from_parent(handler); if (ret < 0) return log_error(-1, "Failed to receive veth names from parent"); diff --git a/src/lxc/network.c b/src/lxc/network.c index 14e5cdab72..4b3b2a2648 100644 --- a/src/lxc/network.c +++ b/src/lxc/network.c @@ -3763,7 +3763,7 @@ int lxc_restore_phys_nics_to_netns(struct lxc_handler *handler) * If we weren't asked to clone a new network namespace, there's * nothing to restore. */ - if (!(handler->ns_clone_flags & CLONE_NEWNET)) + if (!container_uses_namespace(handler, CLONE_NEWNET)) return 0; /* We need CAP_NET_ADMIN in the parent namespace in order to setns() to diff --git a/src/lxc/start.c b/src/lxc/start.c index d8f641a8d7..a34f76a609 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1565,7 +1565,7 @@ static int core_scheduling(struct lxc_handler *handler) if (!conf->sched_core) return log_trace(0, "No new core scheduling domain requested"); - if (!(handler->ns_clone_flags & CLONE_NEWPID)) + if (!container_uses_namespace(handler, CLONE_NEWPID)) return syserror_set(-EINVAL, "Core scheduling currently requires a separate pid namespace"); ret = core_scheduling_cookie_create_threadgroup(handler->pid); @@ -1641,7 +1641,7 @@ static int lxc_spawn(struct lxc_handler *handler) data_sock0 = handler->data_sock[0]; data_sock1 = handler->data_sock[1]; - if (handler->ns_clone_flags & CLONE_NEWNET) { + if (container_uses_namespace(handler, CLONE_NEWNET)) { ret = lxc_find_gateway_addresses(handler); if (ret) { ERROR("Failed to find gateway addresses"); @@ -1685,7 +1685,7 @@ static int lxc_spawn(struct lxc_handler *handler) .exit_signal = SIGCHLD, }; - if (handler->ns_clone_flags & CLONE_NEWCGROUP) { + if (container_uses_namespace(handler, CLONE_NEWCGROUP)) { cgroup_fd = cgroup_unified_fd(cgroup_ops); if (cgroup_fd >= 0) { handler->clone_flags |= CLONE_INTO_CGROUP; @@ -1840,7 +1840,7 @@ static int lxc_spawn(struct lxc_handler *handler) TRACE("Allocated new network namespace id"); /* Create the network configuration. */ - if (handler->ns_clone_flags & CLONE_NEWNET) { + if (container_uses_namespace(handler, CLONE_NEWNET)) { ret = lxc_create_network(handler); if (ret < 0) { ERROR("Failed to create the network"); @@ -1870,7 +1870,7 @@ static int lxc_spawn(struct lxc_handler *handler) goto out_delete_net; } - if (handler->ns_clone_flags & CLONE_NEWNET) { + if (container_uses_namespace(handler, CLONE_NEWNET)) { ret = lxc_network_send_to_child(handler); if (ret < 0) { SYSERROR("Failed to send veth names to child"); @@ -1986,7 +1986,7 @@ static int lxc_spawn(struct lxc_handler *handler) return 0; out_delete_net: - if (handler->ns_clone_flags & CLONE_NEWNET) + if (container_uses_namespace(handler, CLONE_NEWNET)) lxc_delete_network(handler); out_abort: From 9ac7c4895e3f8344c2f789706aca489b32039907 Mon Sep 17 00:00:00 2001 From: Alexander Mikhalitsyn Date: Fri, 26 Jan 2024 17:20:27 +0100 Subject: [PATCH 2/2] tree-wide: use container_uses_namespace() in less trivial cases In our current codebase we have a logical pattern: list_empty(&handler->conf->id_map) *IF AND ONLY IF* container does NOT use user namespace Which is perfectly correct nowadays, but once we (hopefully) get an "isolated user namespaces" stuff ready it won't be the case. It will be perfectly fine to have a user namespace with empty /proc/*/{u,g}id_map files. Nowadays it's also possible, but this kind of a configuration close to useless and nobody actually uses it. No functional changes intended. Signed-off-by: Alexander Mikhalitsyn --- src/lxc/cgroups/cgfsng.c | 2 +- src/lxc/conf.c | 4 ++-- src/lxc/start.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 23e92d6aa6..b4ab0aa697 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -564,7 +564,7 @@ __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops, * monitor is root we can assume that it is privileged enough to remove * the cgroups it created when the container started. */ - if (!list_empty(&handler->conf->id_map) && !handler->am_root) { + if (container_uses_namespace(handler, CLONE_NEWUSER) && !handler->am_root) { struct generic_userns_exec_data wrap = { .conf = handler->conf, .path_prune = ops->container_limit_cgroup, diff --git a/src/lxc/conf.c b/src/lxc/conf.c index c6c4279e2f..d006bccc15 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -620,7 +620,7 @@ int lxc_rootfs_prepare_parent(struct lxc_handler *handler) int ret; const char *path_source; - if (list_empty(&handler->conf->id_map)) + if (!container_uses_namespace(handler, CLONE_NEWUSER)) return 0; if (is_empty_string(rootfs->mnt_opts.userns_path)) @@ -4117,7 +4117,7 @@ static int lxc_rootfs_prepare_child(struct lxc_handler *handler) int dfd_idmapped = -EBADF; int ret; - if (list_empty(&handler->conf->id_map)) + if (!container_uses_namespace(handler, CLONE_NEWUSER)) return 0; if (is_empty_string(rootfs->mnt_opts.userns_path)) diff --git a/src/lxc/start.c b/src/lxc/start.c index a34f76a609..33e4ac94aa 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1400,7 +1400,7 @@ static int do_start(void *data) * we switched to root in the new user namespace further above. Only * drop groups if we can, so ensure that we have necessary privilege. */ - if (list_empty(&handler->conf->id_map)) { + if (!container_uses_namespace(handler, CLONE_NEWUSER)) { #if HAVE_LIBCAP if (lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE)) #endif