Merge pull request #3662 from brauner/2021-02-08/fixes

configure.ac

@@ -644,7 +644,7 @@ AC_CHECK_HEADER([ifaddrs.h],
 AC_HEADER_MAJOR
 
 # Check for some syscalls functions
-AC_CHECK_FUNCS([setns pivot_root sethostname unshare rand_r confstr faccessat gettid memfd_create move_mount open_tree execveat clone3 fsopen fspick fsconfig fsmount, openat2, close_range])
+AC_CHECK_FUNCS([setns pivot_root sethostname unshare rand_r confstr faccessat gettid memfd_create move_mount open_tree execveat clone3 fsopen fspick fsconfig fsmount, openat2, close_range, statvfs])
 AC_CHECK_TYPES([struct open_how], [], [], [[#include <linux/openat2.h>]])
 AC_CHECK_TYPES([struct clone_args], [], [], [[#include <linux/sched.h>]])
 AC_CHECK_MEMBERS([struct clone_args.set_tid],[],[],[[#include <linux/sched.h>]])
@@ -684,7 +684,6 @@ fi
 
 # Check for some functions
 AC_CHECK_LIB(pthread, main)
-AC_CHECK_FUNCS(statvfs)
 AC_CHECK_LIB(util, openpty)
 AC_CHECK_FUNCS([hasmntopt setmntent endmntent utmpxname])
 AC_CHECK_FUNCS([getgrgid_r],

src/lxc/conf.h

@@ -196,10 +196,15 @@ struct lxc_tty_info {
  */
 struct lxc_rootfs {
 	int dfd_host;
-	int dfd_mnt;
-	int dfd_dev;
+
 	char *path;
+	int fd_path_pin;
+
+	int dfd_mnt;
 	char *mount;
+
+	int dfd_dev;
+
 	char buf[PATH_MAX];
 	char *bdev_type;
 	char *options;
@@ -481,7 +486,7 @@ extern struct lxc_conf *current_config;
 __hidden extern int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf, char *argv[]);
 __hidden extern struct lxc_conf *lxc_conf_init(void);
 __hidden extern void lxc_conf_free(struct lxc_conf *conf);
-__hidden extern int pin_rootfs(const char *rootfs);
+__hidden extern int lxc_rootfs_prepare(struct lxc_rootfs *rootfs, bool userns);
 __hidden extern int lxc_map_ids(struct lxc_list *idmap, pid_t pid);
 __hidden extern int lxc_create_tty(const char *name, struct lxc_conf *conf);
 __hidden extern void lxc_delete_tty(struct lxc_tty_info *ttys);
@@ -516,8 +521,6 @@ __hidden extern void turn_into_dependent_mounts(void);
 __hidden extern void suggest_default_idmap(void);
 __hidden extern FILE *make_anonymous_mount_file(struct lxc_list *mount, bool include_nesting_helpers);
 __hidden extern struct lxc_list *sort_cgroup_settings(struct lxc_list *cgroup_settings);
-__hidden extern unsigned long add_required_remount_flags(const char *s, const char *d,
-							 unsigned long flags);
 __hidden extern int run_script(const char *name, const char *section, const char *script, ...);
 __hidden extern int run_script_argv(const char *name, unsigned int hook_version, const char *section,
 				    const char *script, const char *hookname, char **argsin);
@@ -559,4 +562,15 @@ static inline const char *get_rootfs_mnt(const struct lxc_rootfs *rootfs)
 	return !is_empty_string(rootfs->path) ? rootfs->mount : s;
 }
 
+static inline void put_lxc_rootfs(struct lxc_rootfs *rootfs, bool unpin)
+{
+	if (rootfs) {
+		close_prot_errno_disarm(rootfs->dfd_host);
+		close_prot_errno_disarm(rootfs->dfd_mnt);
+		close_prot_errno_disarm(rootfs->dfd_dev);
+		if (unpin)
+			close_prot_errno_disarm(rootfs->fd_path_pin);
+	}
+}
+
 #endif /* __LXC_CONF_H */

src/lxc/mount_utils.c

@@ -11,6 +11,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include "file_utils.h"
 #include "log.h"
 #include "macro.h"
 #include "memory_utils.h"
@@ -18,6 +19,10 @@
 #include "syscall_numbers.h"
 #include "syscall_wrappers.h"
 
+#ifdef HAVE_STATVFS
+#include <sys/statvfs.h>
+#endif
+
 lxc_log_define(mount_utils, lxc);
 
 int mnt_attributes_new(unsigned int old_flags, unsigned int *new_flags)
@@ -239,7 +244,7 @@ int fd_bind_mount(int dfd_from, const char *path_from,
 {
 	__do_close int __fd_from = -EBADF, __fd_to = -EBADF;
 	__do_close int fd_tree_from = -EBADF;
-	unsigned int open_tree_flags = AT_EMPTY_PATH | OPEN_TREE_CLONE | OPEN_TREE_CLONE;
+	unsigned int open_tree_flags = AT_EMPTY_PATH | OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC;
 	int fd_from, fd_to, ret;
 
 	if (!is_empty_string(path_from)) {
@@ -284,3 +289,152 @@ int fd_bind_mount(int dfd_from, const char *path_from,
 	TRACE("Attach detached mount %d to filesystem at %d", fd_tree_from, fd_to);
 	return 0;
 }
+
+int calc_remount_flags_new(int dfd_from, const char *path_from,
+			   __u64 o_flags_from, __u64 resolve_flags_from,
+			   bool remount, unsigned long cur_flags,
+			   unsigned int *new_flags)
+{
+#ifdef HAVE_STATVFS
+	__do_close int fd_from = -EBADF;
+	unsigned int new_required_flags = 0;
+	int ret;
+	struct statvfs sb;
+
+	fd_from = open_at(dfd_from, path_from, o_flags_from, resolve_flags_from, 0);
+	if (fd_from < 0)
+		return log_error_errno(-errno, errno, "Failed to open %d(%s)", dfd_from, maybe_empty(path_from));
+
+	ret = fstatvfs(dfd_from, &sb);
+	if (ret < 0)
+		return log_error_errno(-errno, errno, "Failed to retrieve mount information from %d(%s)", fd_from, maybe_empty(path_from));
+
+	if (remount) {
+		if (sb.f_flag & MS_NOSUID)
+			new_required_flags |= MOUNT_ATTR_NOSUID;
+
+		if (sb.f_flag & MS_NODEV)
+			new_required_flags |= MOUNT_ATTR_NODEV;
+
+		if (sb.f_flag & MS_RDONLY)
+			new_required_flags |= MOUNT_ATTR_RDONLY;
+
+		if (sb.f_flag & MS_NOEXEC)
+			new_required_flags |= MOUNT_ATTR_NOEXEC;
+	}
+
+	if (sb.f_flag & MS_NOATIME)
+		new_required_flags |= MOUNT_ATTR_NOATIME;
+
+	if (sb.f_flag & MS_NODIRATIME)
+		new_required_flags |= MOUNT_ATTR_NODIRATIME;
+
+	if (sb.f_flag & MS_RELATIME)
+		new_required_flags |= MOUNT_ATTR_RELATIME;
+
+	if (sb.f_flag & MS_STRICTATIME)
+		new_required_flags |= MOUNT_ATTR_STRICTATIME;
+
+	*new_flags = (cur_flags | new_required_flags);
+#endif
+	return 0;
+}
+
+int calc_remount_flags_old(int dfd_from, const char *path_from,
+			   __u64 o_flags_from, __u64 resolve_flags_from,
+			   bool remount, unsigned long cur_flags,
+			   unsigned int *old_flags)
+{
+#ifdef HAVE_STATVFS
+	__do_close int fd_from = -EBADF;
+	unsigned int old_required_flags = 0;
+	int ret;
+	struct statvfs sb;
+
+	fd_from = open_at(dfd_from, path_from, o_flags_from, resolve_flags_from, 0);
+	if (fd_from < 0)
+		return log_error_errno(-errno, errno, "Failed to open %d(%s)", dfd_from, maybe_empty(path_from));
+
+	ret = fstatvfs(dfd_from, &sb);
+	if (ret < 0)
+		return log_error_errno(-errno, errno, "Failed to retrieve mount information from %d(%s)", fd_from, maybe_empty(path_from));
+
+	if (remount) {
+		if (sb.f_flag & MS_NOSUID)
+			old_required_flags |= MS_NOSUID;
+
+		if (sb.f_flag & MS_NODEV)
+			old_required_flags |= MS_NODEV;
+
+		if (sb.f_flag & MS_RDONLY)
+			old_required_flags |= MS_RDONLY;
+
+		if (sb.f_flag & MS_NOEXEC)
+			old_required_flags |= MS_NOEXEC;
+	}
+
+	if (sb.f_flag & MS_NOATIME)
+		old_required_flags |= MS_NOATIME;
+
+	if (sb.f_flag & MS_NODIRATIME)
+		old_required_flags |= MS_NODIRATIME;
+
+	if (sb.f_flag & MS_RELATIME)
+		old_required_flags |= MS_RELATIME;
+
+	if (sb.f_flag & MS_STRICTATIME)
+		old_required_flags |= MS_STRICTATIME;
+
+	*old_flags = (cur_flags | old_required_flags);
+#endif
+	return 0;
+}
+
+/* If we are asking to remount something, make sure that any NOEXEC etc are
+ * honored.
+ */
+unsigned long add_required_remount_flags(const char *s, const char *d,
+                                        unsigned long flags)
+{
+#ifdef HAVE_STATVFS
+       int ret;
+       struct statvfs sb;
+       unsigned long required_flags = 0;
+
+       if (!s)
+               s = d;
+
+       if (!s)
+               return flags;
+
+       ret = statvfs(s, &sb);
+       if (ret < 0)
+               return flags;
+
+       if (flags & MS_REMOUNT) {
+               if (sb.f_flag & MS_NOSUID)
+                       required_flags |= MS_NOSUID;
+               if (sb.f_flag & MS_NODEV)
+                       required_flags |= MS_NODEV;
+               if (sb.f_flag & MS_RDONLY)
+                       required_flags |= MS_RDONLY;
+               if (sb.f_flag & MS_NOEXEC)
+                       required_flags |= MS_NOEXEC;
+       }
+
+       if (sb.f_flag & MS_NOATIME)
+               required_flags |= MS_NOATIME;
+       if (sb.f_flag & MS_NODIRATIME)
+               required_flags |= MS_NODIRATIME;
+       if (sb.f_flag & MS_LAZYTIME)
+               required_flags |= MS_LAZYTIME;
+       if (sb.f_flag & MS_RELATIME)
+               required_flags |= MS_RELATIME;
+       if (sb.f_flag & MS_STRICTATIME)
+               required_flags |= MS_STRICTATIME;
+
+       return flags | required_flags;
+#else
+       return flags;
+#endif
+}

src/lxc/mount_utils.h

@@ -207,4 +207,20 @@ static inline bool new_mount_api(void)
 	return supported == 1;
 }
 
+__hidden extern int calc_remount_flags_new(int dfd_from, const char *path_from,
+					   __u64 o_flags_from,
+					   __u64 resolve_flags_from,
+					   bool remount, unsigned long cur_flags,
+					   unsigned int *new_flags);
+
+__hidden extern int calc_remount_flags_old(int dfd_from, const char *path_from,
+					   __u64 o_flags_from,
+					   __u64 resolve_flags_from,
+					   bool remount, unsigned long cur_flags,
+					   unsigned int *old_flags);
+
+__hidden extern unsigned long add_required_remount_flags(const char *s,
+							 const char *d,
+							 unsigned long flags);
+
 #endif /* __LXC_MOUNT_UTILS_H */

src/lxc/start.c

@@ -618,7 +618,6 @@ out_sigfd:
 
 void lxc_put_handler(struct lxc_handler *handler)
 {
-	close_prot_errno_disarm(handler->pinfd);
 	close_prot_errno_disarm(handler->pidfd);
 	close_prot_errno_disarm(handler->sigfd);
 	lxc_put_nsfds(handler);
@@ -660,7 +659,6 @@ struct lxc_handler *lxc_init_handler(struct lxc_handler *old,
 	handler->data_sock[0] = -EBADF;
 	handler->data_sock[1] = -EBADF;
 	handler->monitor_status_fd = -EBADF;
-	handler->pinfd = -EBADF;
 	handler->pidfd = -EBADF;
 	handler->sigfd = -EBADF;
 	handler->state_socket_pair[0] = -EBADF;
@@ -925,6 +923,8 @@ void lxc_end(struct lxc_handler *handler)
 		cgroup_ops->monitor_destroy(cgroup_ops, handler);
 	}
 
+	put_lxc_rootfs(&handler->conf->rootfs, true);
+
 	if (handler->conf->reboot == REBOOT_NONE) {
 		/* For all new state clients simply close the command socket.
 		 * This will inform all state clients that the container is
@@ -1066,9 +1066,6 @@ static int do_start(void *data)
 		goto out_warn_father;
 	}
 
-	/* Don't leak the pinfd to the container. */
-	close_prot_errno_disarm(handler->pinfd);
-
 	if (!lxc_sync_wait_parent(handler, START_SYNC_STARTUP))
 		goto out_warn_father;
 
@@ -1666,10 +1663,10 @@ static int lxc_spawn(struct lxc_handler *handler)
 	 * it readonly.
 	 * If the container is unprivileged then skip rootfs pinning.
 	 */
-	if (!wants_to_map_ids) {
-		handler->pinfd = pin_rootfs(conf->rootfs.path);
-		if (handler->pinfd == -EBADF)
-			INFO("Failed to pin the rootfs for container \"%s\"", handler->name);
+	ret = lxc_rootfs_prepare(&conf->rootfs, wants_to_map_ids);
+	if (ret) {
+		ERROR("Failed to handle rootfs pinning for container \"%s\"", handler->name);
+		goto out_delete_net;
 	}
 
 	/* Create a process in a new set of namespaces. */
@@ -2001,7 +1998,6 @@ out_abort:
 
 out_sync_fini:
 	lxc_sync_fini(handler);
-	close_prot_errno_disarm(handler->pinfd);
 
 	return -1;
 }
@@ -2118,8 +2114,6 @@ int __lxc_start(struct lxc_handler *handler, struct lxc_operations *ops,
 	if (ret < 0)
 		ERROR("Failed to move physical network devices back to parent network namespace");
 
-	close_prot_errno_disarm(handler->pinfd);
-
 	lxc_monitor_send_exit_code(name, status, handler->lxcpath);
 	lxc_error_set_and_log(handler->pid, status);
 	if (error_num)

src/lxc/start.h

@@ -43,9 +43,6 @@ struct lxc_handler {
 		__aligned_u64 clone_flags;
 	};
 
-	/* File descriptor to pin the rootfs for privileged containers. */
-	int pinfd;
-
 	/* Signal file descriptor. */
 	int sigfd;
 

src/lxc/storage/dir.c

@@ -11,6 +11,7 @@
 #include "log.h"
 #include "macro.h"
 #include "memory_utils.h"
+#include "mount_utils.h"
 #include "storage.h"
 #include "utils.h"
 

src/tests/lxc-test-unpriv

@@ -66,11 +66,12 @@ which newuidmap >/dev/null 2>&1 || { echo "'newuidmap' command is missing" >&2;
 
 DONE=0
 KNOWN_RELEASES="precise trusty xenial yakkety zesty"
+UNPRIV_LOG=$(mktemp --dry-run)
 cleanup() {
 	cd /
 
-	run_cmd lxc-stop -n c2 -k || true
-	run_cmd lxc-stop -n c1 -k || true
+	run_cmd lxc-stop -n c2 -k -l trace -o "${UNPRIV_LOG}" || true
+	run_cmd lxc-stop -n c1 -k -l trace -o "${UNPRIV_LOG}" || true
 	pkill -u $(id -u $TUSER) -9 || true
 
 	sed -i '/lxcunpriv/d' /run/lxc/nics /etc/lxc/lxc-usernet
@@ -81,6 +82,8 @@ cleanup() {
 	deluser $TUSER
 
 	if [ $DONE -eq 0 ]; then
+		cat "${UNPRIV_LOG}"
+		rm -f "${UNPRIV_LOG}" || true
 		echo "FAIL"
 		exit 1
 	fi
@@ -173,45 +176,45 @@ run_cmd mkdir -p $HDIR/.cache/lxc
     cp -R /var/cache/lxc/download $HDIR/.cache/lxc && \
     chown -R $TUSER: $HDIR/.cache/lxc
 
-run_cmd lxc-create -t download -n c1 -- -d ubuntu -r $release -a $ARCH
+run_cmd lxc-create -t download -n c1 -l trace -o "${UNPRIV_LOG}" -- -d ubuntu -r $release -a $ARCH
 
 # Make sure we can start it - twice
 
 for count in `seq 1 2`; do
-    run_cmd lxc-start -n c1 -d
+    run_cmd lxc-start -n c1 -d -l trace -o "${UNPRIV_LOG}"
 
-    p1=$(run_cmd lxc-info -n c1 -p -H)
+    p1=$(run_cmd lxc-info -n c1 -p -H -l trace -o "${UNPRIV_LOG}")
     [ "$p1" != "-1" ] || { echo "Failed to start container c1 (run $count)"; false; }
 
-    run_cmd lxc-info -n c1
-    run_cmd lxc-attach -n c1 -- /bin/true
+    run_cmd lxc-info -n c1 -l trace -o "${UNPRIV_LOG}"
+    run_cmd lxc-attach -n c1 -l trace -o "${UNPRIV_LOG}" -- /bin/true
 
-    run_cmd lxc-stop -n c1 -k
+    run_cmd lxc-stop -n c1 -k -l trace -o "${UNPRIV_LOG}"
 done
 
-run_cmd lxc-copy -s -n c1 -N c2
-run_cmd lxc-start -n c2 -d
-p1=$(run_cmd lxc-info -n c2 -p -H)
+run_cmd lxc-copy -s -n c1 -N c2 -l trace -o "${UNPRIV_LOG}"
+run_cmd lxc-start -n c2 -d -l trace -o "${UNPRIV_LOG}"
+p1=$(run_cmd lxc-info -n c2 -p -H -l trace -o "${UNPRIV_LOG}")
 [ "$p1" != "-1" ] || { echo "Failed to start container c2"; false; }
 
-run_cmd lxc-stop -n c2 -k
+run_cmd lxc-stop -n c2 -k -l trace -o "${UNPRIV_LOG}"
 
 if which cgm >/dev/null 2>&1; then
     echo "Testing containers under different cgroups per subsystem"
     run_cmd cgm create freezer x1/x2
     cgm movepid freezer x1 $$
-    run_cmd lxc-start -n c1 -d
-    p1=$(run_cmd lxc-info -n c1 -p -H)
+    run_cmd lxc-start -n c1 -d -l trace -o "${UNPRIV_LOG}"
+    p1=$(run_cmd lxc-info -n c1 -p -H -l trace -o "${UNPRIV_LOG}")
     [ "$p1" != "-1" ] || { echo "Failed to start container c1"; false; }
-    run_cmd lxc-info -n c1
-    run_cmd lxc-attach -n c1 -- /bin/true
-    run_cmd lxc-cgroup -n c1 freezer.state
+    run_cmd lxc-info -n c1 -l trace -o "${UNPRIV_LOG}"
+    run_cmd lxc-attach -n c1 -l trace -o "${UNPRIV_LOG}" -- /bin/true
+    run_cmd lxc-cgroup -n c1 freezer.state -l trace -o "${UNPRIV_LOG}"
 
     echo "Testing lxc-attach and lxc-cgroup from different cgroup"
     cgm movepid freezer x2 $$
-    run_cmd lxc-attach -n c1 -- /bin/true
-    run_cmd lxc-cgroup -n c1 freezer.state
-    run_cmd lxc-cgroup -n c1 memory.limit_in_bytes
+    run_cmd lxc-attach -n c1 -l trace -o "${UNPRIV_LOG}" -- /bin/true
+    run_cmd lxc-cgroup -n c1 -l trace -o "${UNPRIV_LOG}" freezer.state
+    run_cmd lxc-cgroup -n c1 -l trace -o "${UNPRIV_LOG}" memory.limit_in_bytes
 fi
 
 DONE=1