diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e7318151fe..291fa58223 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,6 +30,14 @@ jobs: run: | ${CC} --version + - name: Kernel version + run: | + uname -a + + - name: Mount table + run: | + findmnt + - name: Build env: CC: ${{ matrix.compiler }} diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml index 7003729edd..9ea82a3509 100644 --- a/.github/workflows/coverity.yml +++ b/.github/workflows/coverity.yml @@ -24,6 +24,20 @@ jobs: sudo apt-get install -qq gcc clang sudo apt-get install -qq libapparmor-dev libcap-dev libseccomp-dev libselinux1-dev linux-libc-dev docbook2x + - name: Compiler version + env: + CC: ${{ matrix.compiler }} + run: | + ${CC} --version + + - name: Kernel version + run: | + uname -a + + - name: Mount table + run: | + findmnt + - name: Run coverity run: | # Configure diff --git a/.github/workflows/sanitizers.sh b/.github/workflows/sanitizers.sh index 680ac796a9..ee650a6cfc 100755 --- a/.github/workflows/sanitizers.sh +++ b/.github/workflows/sanitizers.sh @@ -18,9 +18,43 @@ apt-get install --yes --no-install-recommends \ libpam0g-dev libseccomp-dev libselinux1-dev libtool linux-libc-dev \ llvm lsb-release make openssl pkg-config python3-all-dev \ python3-setuptools rsync squashfs-tools uidmap unzip uuid-runtime \ - wget xz-utils + wget xz-utils systemd-coredump +apt-get remove --yes lxc-utils liblxc-common liblxc1 liblxc-dev + +ARGS="--enable-sanitizers \ + --prefix=/usr/ \ + --disable-no-undefined \ + --build=x86_64-linux-gnu \ + --includedir=\${prefix}/include \ + --mandir=\${prefix}/share/man \ + --infodir=\${prefix}/share/info \ + --sysconfdir=/etc \ + --localstatedir=/var \ + --disable-silent-rules \ + --libdir=\${prefix}/lib/x86_64-linux-gnu \ + --libexecdir=\${prefix}/lib/x86_64-linux-gnu \ + --disable-maintainer-mode \ + --disable-dependency-tracking \ + --libdir=\${prefix}/lib/x86_64-linux-gnu \ + --libexecdir=\${prefix}/lib/x86_64-linux-gnu \ + --with-rootfs-path=\${prefix}/lib/x86_64-linux-gnu/lxc \ + --enable-doc \ + --disable-rpath \ + --with-distro=ubuntu \ + --enable-commands \ + --enable-pam \ + --enable-tests \ + --enable-memfd-rexec \ + --disable-static-binaries \ + --enable-static \ + --enable-silent-rules \ + --enable-apparmor \ + --enable-capabilities \ + --enable-seccomp \ + --enable-selinux \ + --disable-liburing \ + --enable-werror" -ARGS="--enable-sanitizers --enable-tests --prefix=/usr/ --sysconfdir=/etc/ --localstatedir=/var/ --disable-no-undefined" case "$CC" in clang*) ARGS="$ARGS --enable-fuzzers" esac diff --git a/.github/workflows/sanitizers.yml b/.github/workflows/sanitizers.yml index 4c66a47c46..76339e19c9 100644 --- a/.github/workflows/sanitizers.yml +++ b/.github/workflows/sanitizers.yml @@ -15,6 +15,20 @@ jobs: - name: Checkout code uses: actions/checkout@v2 + - name: Compiler version + env: + CC: ${{ matrix.compiler }} + run: | + ${CC} --version + + - name: Kernel version + run: | + uname -a + + - name: Mount table + run: | + findmnt + - name: Build run: | sudo CC=${{ matrix.compiler }} CXX=${{ matrix.compiler }}++ .github/workflows/sanitizers.sh diff --git a/config/tls.m4 b/config/tls.m4 deleted file mode 100644 index cd032c9d70..0000000000 --- a/config/tls.m4 +++ /dev/null @@ -1,14 +0,0 @@ -# See if we have working TLS. We only check to see if it compiles, and that -# the resulting program actually runs, not whether the resulting TLS variables -# work properly; that check is done at runtime, since we can run binaries -# compiled with __thread on systems without TLS. -AC_DEFUN([LXC_CHECK_TLS], -[ - AC_MSG_CHECKING(for TLS) - AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ static __thread int val; int main() { return 0; } ]])],[have_tls=yes],[have_tls=no],[have_tls=no ]) - AC_MSG_RESULT($have_tls) - if test "$have_tls" = "yes"; then - AC_DEFINE([HAVE_TLS],[1],[Define if the compiler supports __thread]) - AC_DEFINE([thread_local],[__thread],[Define to the compiler TLS keyword]) - fi -]) diff --git a/configure.ac b/configure.ac index 0ae5ad6fcb..581f0d7d49 100644 --- a/configure.ac +++ b/configure.ac @@ -500,7 +500,9 @@ if test "x$enable_fuzzers" = "xyes"; then CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[ \ -DRUN_ON_OSS_FUZZ=1]) fi -else +fi + +if test "x$enable_fuzzers" = "xno" -a "x$enable_sanitizers" = "xno"; then CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[-flto=thin]) fi AC_SUBST(AM_CFLAGS) @@ -775,9 +777,6 @@ AC_CHECK_TYPES([struct rtnl_link_stats64], [], [], [[#include ] AX_PTHREAD AC_SEARCH_LIBS(clock_gettime, [rt]) -# See if we support thread-local storage. -LXC_CHECK_TLS - # Hardening flags CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[ \ -fPIE \ diff --git a/src/lxc/compiler.h b/src/lxc/compiler.h index 4d62323822..907941d981 100644 --- a/src/lxc/compiler.h +++ b/src/lxc/compiler.h @@ -5,8 +5,14 @@ #include "config.h" -#include +#include +#include +#include #include +#include +#include +#include +#include #ifndef thread_local #if __STDC_VERSION__ >= 201112L && \ diff --git a/src/lxc/conf.c b/src/lxc/conf.c index fe54718a95..e656f63bbd 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -5505,11 +5505,20 @@ int userns_exec_mapped_root(const char *path, int path_fd, close_prot_errno_disarm(sock_fds[0]); - if (!lxc_switch_uid_gid(0, 0)) + if (!lxc_drop_groups() && errno != EPERM) _exit(EXIT_FAILURE); - if (!lxc_drop_groups()) + ret = setresgid(0, 0, 0); + if (ret < 0) { + SYSERROR("Failed to setresgid(0, 0, 0)"); + _exit(EXIT_FAILURE); + } + + ret = setresuid(0, 0, 0); + if (ret < 0) { + SYSERROR("Failed to setresuid(0, 0, 0)"); _exit(EXIT_FAILURE); + } ret = fchown(target_fd, 0, st.st_gid); if (ret) { @@ -5557,9 +5566,12 @@ int userns_exec_mapped_root(const char *path, int path_fd, /* Wait for child to finish. */ if (pid < 0) + return log_error(-1, "Failed to create child process"); + + if (!wait_exited(pid)) return -1; - return wait_for_pid(pid); + return 0; } /* not thread-safe, do not use from api without first forking */ diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 96fdb3fd22..5dbfc0fc5c 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -170,41 +170,32 @@ static int ongoing_create(struct lxc_container *c) return LXC_CREATE_INCOMPLETE; } -static int create_partial(struct lxc_container *c) +static int create_partial(int fd_rootfs, struct lxc_container *c) { - __do_free char *path = NULL; - int fd, ret; - size_t len; + __do_close int fd_partial = -EBADF; + int ret; struct flock lk = {0}; - /* $lxcpath + '/' + $name + '/partial' + \0 */ - len = strlen(c->config_path) + 1 + strlen(c->name) + 1 + strlen(LXC_PARTIAL_FNAME) + 1; - path = must_realloc(NULL, len); - ret = strnprintf(path, len, "%s/%s/%s", c->config_path, c->name, LXC_PARTIAL_FNAME); - if (ret < 0) - return -1; - - fd = open(path, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0000); - if (fd < 0) - return -1; + fd_partial = openat(fd_rootfs, LXC_PARTIAL_FNAME, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0000); + if (fd_partial < 0) + return syserror("errno(%d) - Failed to create \"%d/" LXC_PARTIAL_FNAME "\" to mark container as partially created", errno, fd_rootfs); lk.l_type = F_WRLCK; lk.l_whence = SEEK_SET; - ret = fcntl(fd, F_OFD_SETLKW, &lk); + ret = fcntl(fd_partial, F_OFD_SETLKW, &lk); if (ret < 0) { if (errno == EINVAL) { - ret = flock(fd, LOCK_EX); + ret = flock(fd_partial, LOCK_EX); if (ret == 0) - return fd; + return move_fd(fd_partial); } - SYSERROR("Failed to lock partial file %s", path); - close(fd); - return -1; + return syserror("Failed to lock partial file \"%d/" LXC_PARTIAL_FNAME"\"", fd_rootfs); } - return fd; + TRACE("Created \"%d/" LXC_PARTIAL_FNAME "\" to mark container as partially created", fd_rootfs); + return move_fd(fd_partial); } static void remove_partial(struct lxc_container *c, int fd) @@ -452,7 +443,26 @@ static rettype fnname(struct lxc_container *c, t1 a1, t2 a2, t3 a3) \ return ret; \ } -#define WRAP_API_6(rettype, fnname, t1, t2, t3, t4, t5, t6) \ +#define WRAP_API_5(rettype, fnname, t1, t2, t3, t4, t5) \ +static rettype fnname(struct lxc_container *c, t1 a1, t2 a2, t3 a3, \ + t4 a4, t5 a5) \ +{ \ + rettype ret; \ + bool reset_config = false; \ + \ + if (!current_config && c && c->lxc_conf) { \ + current_config = c->lxc_conf; \ + reset_config = true; \ + } \ + \ + ret = do_##fnname(c, a1, a2, a3, a4, a5); \ + if (reset_config) \ + current_config = NULL; \ + \ + return ret; \ +} + +#define WRAP_API_6(rettype, fnname, t1, t2, t3, t4, t5, t6) \ static rettype fnname(struct lxc_container *c, t1 a1, t2 a2, t3 a3, \ t4 a4, t5 a5, t6 a6) \ { \ @@ -1203,32 +1213,31 @@ WRAP_API(bool, lxcapi_stop) static int do_create_container_dir(const char *path, struct lxc_conf *conf) { - int lasterr; + __do_close int fd_rootfs = -EBADF; int ret = -1; mode_t mask = umask(0002); ret = mkdir(path, 0770); - lasterr = errno; umask(mask); - errno = lasterr; - if (ret) { - if (errno != EEXIST) - return -1; + if (ret < 0 && errno != EEXIST) + return -errno; - ret = 0; - } + fd_rootfs = open_at(-EBADF, path, O_DIRECTORY | O_CLOEXEC, PROTECT_LOOKUP_ABSOLUTE_WITH_SYMLINKS, 0); + if (fd_rootfs < 0) + return syserror("Failed to open container directory \"%d(%s)\"", fd_rootfs, path); - if (!list_empty(&conf->id_map)) { - ret = chown_mapped_root(path, conf); - if (ret < 0) - ret = -1; - } + if (list_empty(&conf->id_map)) + return move_fd(fd_rootfs); - return ret; + ret = userns_exec_mapped_root(NULL, fd_rootfs, conf); + if (ret < 0) + return syserror_ret(-1, "Failed to chown rootfs \"%s\"", path); + + return move_fd(fd_rootfs); } /* Create the standard expected container dir. */ -static bool create_container_dir(struct lxc_container *c) +static int create_container_dir(struct lxc_container *c) { __do_free char *s = NULL; int ret; @@ -1237,13 +1246,13 @@ static bool create_container_dir(struct lxc_container *c) len = strlen(c->config_path) + strlen(c->name) + 2; s = malloc(len); if (!s) - return false; + return ret_errno(ENOMEM); ret = strnprintf(s, len, "%s/%s", c->config_path, c->name); if (ret < 0) - return false; + return -errno; - return do_create_container_dir(s, c->lxc_conf) == 0; + return do_create_container_dir(s, c->lxc_conf); } /* do_storage_create: thin wrapper around storage_create(). Like @@ -1769,15 +1778,16 @@ static void lxcapi_clear_config(struct lxc_container *c) * @argv: the arguments to pass to the template, terminated by NULL. If no * arguments, you can just pass NULL. */ -static bool do_lxcapi_create(struct lxc_container *c, const char *t, - const char *bdevtype, struct bdev_specs *specs, - int flags, char *const argv[]) +static bool __lxcapi_create(struct lxc_container *c, const char *t, + const char *bdevtype, struct bdev_specs *specs, + int flags, char *const argv[]) { + __do_close int fd_rootfs = -EBADF; __do_free char *path_template = NULL; int partial_fd; mode_t mask; pid_t pid; - bool ret = false, rootfs_managed = true; + bool bret = false, rootfs_managed = true; if (!c) return false; @@ -1785,7 +1795,7 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, if (t) { path_template = get_template_path(t); if (!path_template) - return syserror_set(ENOENT, "Template \"%s\" not found", t); + return log_error(false, "Template \"%s\" not found", t); } /* If a template is passed in, and the rootfs already is defined in the @@ -1794,15 +1804,17 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, */ if (do_lxcapi_is_defined(c) && c->lxc_conf && c->lxc_conf->rootfs.path && access(c->lxc_conf->rootfs.path, F_OK) == 0 && path_template) - return syserror_set(EEXIST, "Container \"%s\" already exists in \"%s\"", c->name, c->config_path); + return log_error(false, "Container \"%s\" already exists in \"%s\"", + c->name, c->config_path); if (!c->lxc_conf && !do_lxcapi_load_config(c, lxc_global_config_value("lxc.default_config"))) - return syserror_set(EINVAL, "Failed to load default configuration file %s", - lxc_global_config_value("lxc.default_config")); + return log_error(false, "Failed to load default configuration file %s", + lxc_global_config_value("lxc.default_config")); - if (!create_container_dir(c)) - return syserror_set(EINVAL, "Failed to create container %s", c->name); + fd_rootfs = create_container_dir(c); + if (fd_rootfs < 0) + return log_error(false, "Failed to create container %s", c->name); if (c->lxc_conf->rootfs.path) rootfs_managed = false; @@ -1817,13 +1829,16 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, ERROR("Failed to save initial config for \"%s\"", c->name); goto out; } - ret = true; + + bret = true; goto out; } /* Rootfs passed into configuration, but does not exist. */ - if (c->lxc_conf->rootfs.path && access(c->lxc_conf->rootfs.path, F_OK) != 0) + if (c->lxc_conf->rootfs.path && access(c->lxc_conf->rootfs.path, F_OK) != 0) { + ERROR("The rootfs \"%s\" does not exist", c->lxc_conf->rootfs.path); goto out; + } if (do_lxcapi_is_defined(c) && c->lxc_conf->rootfs.path && !path_template) { /* Rootfs already existed, user just wanted to save the loaded @@ -1832,14 +1847,16 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, if (!c->save_config(c, NULL)) ERROR("Failed to save initial config for \"%s\"", c->name); - ret = true; + bret = true; goto out; } - /* Mark that this container is being created */ - partial_fd = create_partial(c); - if (partial_fd < 0) + /* Mark that this container as being created */ + partial_fd = create_partial(fd_rootfs, c); + if (partial_fd < 0) { + SYSERROR("Failed to mark container as being partially created"); goto out; + } /* No need to get disk lock bc we have the partial lock. */ @@ -1881,7 +1898,7 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, _exit(EXIT_SUCCESS); } - if (wait_for_pid(pid) != 0) + if (!wait_exited(pid)) goto out_unlock; /* Reload config to get the rootfs. */ @@ -1906,14 +1923,14 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, } } - ret = load_config_locked(c, c->configfile); + bret = load_config_locked(c, c->configfile); out_unlock: umask(mask); remove_partial(c, partial_fd); out: - if (!ret) { + if (!bret) { bool reset_managed = c->lxc_conf->rootfs.managed; /* @@ -1926,22 +1943,19 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t, c->lxc_conf->rootfs.managed = reset_managed; } - return ret; + return bret; } -static bool lxcapi_create(struct lxc_container *c, const char *t, - const char *bdevtype, struct bdev_specs *specs, - int flags, char *const argv[]) +static bool do_lxcapi_create(struct lxc_container *c, const char *t, + const char *bdevtype, struct bdev_specs *specs, + int flags, char *const argv[]) { - bool ret; - - current_config = c ? c->lxc_conf : NULL; - - ret = do_lxcapi_create(c, t, bdevtype, specs, flags, argv); - current_config = NULL; - return ret; + return __lxcapi_create(c, t, bdevtype, specs, flags, argv); } +WRAP_API_5(bool, lxcapi_create, const char *, const char *, + struct bdev_specs *, int, char *const *) + static bool do_lxcapi_reboot(struct lxc_container *c) { __do_close int pidfd = -EBADF; @@ -2169,7 +2183,7 @@ static bool lxcapi_createl(struct lxc_container *c, const char *t, goto out; } - bret = do_lxcapi_create(c, t, bdevtype, specs, flags, args); + bret = __lxcapi_create(c, t, bdevtype, specs, flags, args); out: free(args); @@ -2605,28 +2619,27 @@ WRAP_API_3(int, lxcapi_get_keys, const char *, char *, int) static bool do_lxcapi_save_config(struct lxc_container *c, const char *alt_file) { - int fd, lret; - bool ret = false, need_disklock = false; + __do_close int fd_config = -EBADF, fd_rootfs = -EBADF; + int lret = -1; + bool bret = false, need_disklock = false; if (!alt_file) alt_file = c->configfile; if (!alt_file) - return false; + return log_error(false, "No config file found"); /* If we haven't yet loaded a config, load the stock config. */ if (!c->lxc_conf) { if (!do_lxcapi_load_config(c, lxc_global_config_value("lxc.default_config"))) { - ERROR("Error loading default configuration file %s " - "while saving %s", - lxc_global_config_value("lxc.default_config"), - c->name); - return false; + return log_error(false, "Error loading default configuration file %s while saving %s", + lxc_global_config_value("lxc.default_config"), c->name); } } - if (!create_container_dir(c)) - return false; + fd_rootfs = create_container_dir(c); + if (fd_rootfs < 0) + return log_error(false, "Failed to create container directory"); /* If we're writing to the container's config file, take the disk lock. * Otherwise just take the memlock to protect the struct lxc_container @@ -2640,19 +2653,23 @@ static bool do_lxcapi_save_config(struct lxc_container *c, const char *alt_file) else lret = container_mem_lock(c); if (lret) - return false; + return log_error(false, "Failed to acquire lock"); - fd = open(alt_file, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, - S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP); - if (fd < 0) + fd_config = open(alt_file, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, + S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP); + if (fd_config < 0) { + SYSERROR("Failed to open config file \"%s\"", alt_file); goto on_error; + } - lret = write_config(fd, c->lxc_conf); - close(fd); - if (lret < 0) + lret = write_config(fd_config, c->lxc_conf); + if (lret < 0) { + SYSERROR("Failed to write config file \"%s\"", alt_file); goto on_error; + } - ret = true; + bret = true; + TRACE("Saved config file \"%s\"", alt_file); on_error: if (need_disklock) @@ -2660,7 +2677,7 @@ static bool do_lxcapi_save_config(struct lxc_container *c, const char *alt_file) else container_mem_unlock(c); - return ret; + return bret; } WRAP_API_1(bool, lxcapi_save_config, const char *) @@ -3742,17 +3759,18 @@ only rootfs gets converted (copied/snapshotted) on clone. static int create_file_dirname(char *path, struct lxc_conf *conf) { - char *p = strrchr(path, '/'); - int ret = -1; + __do_close int fd_rootfs = -EBADF; + char *p; + p = strrchr(path, '/'); if (!p) return -1; *p = '\0'; - ret = do_create_container_dir(path, conf); + fd_rootfs = do_create_container_dir(path, conf); *p = '/'; - return ret; + return fd_rootfs >= 0; } static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char *newname, diff --git a/src/lxc/utils.c b/src/lxc/utils.c index bc8a2b0c30..b2cfb865d6 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -329,6 +329,24 @@ int lxc_wait_for_pid_status(pid_t pid) return status; } +bool wait_exited(pid_t pid) +{ + int status; + + status = lxc_wait_for_pid_status(pid); + if (status < 0) + return log_error(false, "Failed to reap on child process %d", pid); + if (WIFSIGNALED(status)) + return log_error(false, "Child process %d terminated by signal %d", pid, WTERMSIG(status)); + if (!WIFEXITED(status)) + return log_error(false, "Child did not termiate correctly"); + if (WEXITSTATUS(status)) + return log_error(false, "Child terminated with error %d", WEXITSTATUS(status)); + + TRACE("Reaped child process %d", pid); + return true; +} + #ifdef HAVE_OPENSSL #include diff --git a/src/lxc/utils.h b/src/lxc/utils.h index 7b10172650..87feeed767 100644 --- a/src/lxc/utils.h +++ b/src/lxc/utils.h @@ -81,6 +81,7 @@ static inline void __auto_lxc_pclose__(struct lxc_popen_FILE **f) __hidden extern int wait_for_pid(pid_t pid); __hidden extern int lxc_wait_for_pid_status(pid_t pid); __hidden extern int wait_for_pidfd(int pidfd); +__hidden extern bool wait_exited(pid_t pid); #if HAVE_OPENSSL __hidden extern int sha1sum_file(char *fnam, unsigned char *md_value, unsigned int *md_len);