The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxcfs/pull/394
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From bea116fc4c685436323370c75311661b7d1ce81d Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:27:58 +0200 Subject: [PATCH 1/7] cgroup_fuse: do not double-close Fixes: Coverity 355703. Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/cgroup_fuse.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/cgroup_fuse.c b/src/cgroup_fuse.c index e164910..d090873 100644 --- a/src/cgroup_fuse.c +++ b/src/cgroup_fuse.c @@ -745,22 +745,22 @@ __lxcfs_fuse_ops int cg_mkdir(const char *path, mode_t mode) static bool recursive_rmdir(const char *dirname, int fd, const int cfd) { - struct dirent *direntp; - DIR *dir; + __do_close int dupfd = -EBADF; + __do_closedir DIR *dir = NULL; bool ret = false; + struct dirent *direntp; char pathname[MAXPATHLEN]; - int dupfd; - dupfd = dup(fd); // fdopendir() does bad things once it uses an fd. + dupfd = dup(fd); if (dupfd < 0) return false; dir = fdopendir(dupfd); if (!dir) { lxcfs_debug("Failed to open %s: %s.\n", dirname, strerror(errno)); - close(dupfd); return false; } + move_fd(dupfd); while ((direntp = readdir(dir))) { struct stat mystat; @@ -787,18 +787,12 @@ static bool recursive_rmdir(const char *dirname, int fd, const int cfd) } ret = true; - if (closedir(dir) < 0) { - lxcfs_error("Failed to close directory %s: %s\n", dirname, strerror(errno)); - ret = false; - } if (unlinkat(cfd, dirname, AT_REMOVEDIR) < 0) { lxcfs_debug("Failed to delete %s: %s.\n", dirname, strerror(errno)); ret = false; } - close(dupfd); - return ret; } From c7846edb2410d4479816c2b14d3d492a5685d333 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:31:12 +0200 Subject: [PATCH 2/7] lxcfs: remove fl.* prefix Fixes: Coverity 355708. Fixes: Coverity 355702. Fixes: Coverity 355729. Fixes: Coverity 355735. Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxcfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lxcfs.c b/src/lxcfs.c index 4dfb6b9..6414235 100644 --- a/src/lxcfs.c +++ b/src/lxcfs.c @@ -1029,10 +1029,10 @@ static int set_pidfile(char *pidfile) char buf[INTTYPE_TO_STRLEN(long)]; int ret; struct flock fl = { - fl.l_type = F_WRLCK, - fl.l_whence = SEEK_SET, - fl.l_start = 0, - fl.l_len = 0, + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 0, }; fd = open(pidfile, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR | O_CLOEXEC); From a3873e61287d8b7790db53a10f45c144a058ad26 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:34:42 +0200 Subject: [PATCH 3/7] cgroup_fuse: s/clone/lxcfs_clone/g Fixes: Coverity 355713. Fixes: Coverity 355723. Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/bindings.c | 2 +- src/bindings.h | 2 ++ src/cgroup_fuse.c | 8 ++------ 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index 81bebdc..18f47c5 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -337,7 +337,7 @@ static int send_creds_clone_wrapper(void *arg) * stack sizes: 8MB. */ #define __LXCFS_STACK_SIZE (8 * 1024 * 1024) -static pid_t lxcfs_clone(int (*fn)(void *), void *arg, int flags) +pid_t lxcfs_clone(int (*fn)(void *), void *arg, int flags) { pid_t ret; void *stack; diff --git a/src/bindings.h b/src/bindings.h index ee693af..28543dc 100644 --- a/src/bindings.h +++ b/src/bindings.h @@ -103,4 +103,6 @@ static inline int install_signal_handler(int signo, return sigaction(signo, &action, NULL); } +extern pid_t lxcfs_clone(int (*fn)(void *), void *arg, int flags); + #endif /* __LXCFS_BINDINGS_H */ diff --git a/src/cgroup_fuse.c b/src/cgroup_fuse.c index d090873..2d37a59 100644 --- a/src/cgroup_fuse.c +++ b/src/cgroup_fuse.c @@ -1223,10 +1223,8 @@ static void pid_to_ns_wrapper(int sock, pid_t tpid) .tpid = tpid, .wrapped = &pid_to_ns }; - size_t stack_size = sysconf(_SC_PAGESIZE); - void *stack = alloca(stack_size); - cpid = clone(pid_ns_clone_wrapper, stack + stack_size, SIGCHLD, &args); + cpid = lxcfs_clone(pid_ns_clone_wrapper, &args, 0); if (cpid < 0) _exit(1); @@ -1562,10 +1560,8 @@ static void pid_from_ns_wrapper(int sock, pid_t tpid) .tpid = tpid, .wrapped = &pid_from_ns }; - size_t stack_size = sysconf(_SC_PAGESIZE); - void *stack = alloca(stack_size); - cpid = clone(pid_ns_clone_wrapper, stack + stack_size, SIGCHLD, &args); + cpid = lxcfs_clone(pid_ns_clone_wrapper, &args, 0); if (cpid < 0) _exit(1); From a8fcc3febf1a10a99411ae8d8021415d9b14a3db Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:39:05 +0200 Subject: [PATCH 4/7] bindings: do not falsely return This may also explain some of the performance regressions that Blub\0 reported. Fixes: Coverity 355716. Cc: Wolfgang Bumiller <w.bumil...@proxmox.com> Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/bindings.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bindings.c b/src/bindings.c index 18f47c5..8ae8605 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -275,7 +275,7 @@ static void save_initpid(ino_t pidns_inode, pid_t pid) return; entry = malloc(sizeof(*entry)); - if (entry) + if (!entry) return; ino_hash = HASH(entry->ino); From d186570ef55f29bfd7bebd931cace9a726513421 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:43:00 +0200 Subject: [PATCH 5/7] cgroup_fuse: be cautios when dereferencing d->controller Fixes: Coverity 355712. Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/cgroup_fuse.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/cgroup_fuse.c b/src/cgroup_fuse.c index 2d37a59..353e04f 100644 --- a/src/cgroup_fuse.c +++ b/src/cgroup_fuse.c @@ -1971,6 +1971,11 @@ __lxcfs_fuse_ops int cg_readdir(const char *path, void *buf, return 0; } + if (!d->controller) { + ret = -EINVAL; + goto out; + } + if (!cgfs_list_keys(d->controller, d->cgroup, &list)) { // not a valid cgroup ret = -EINVAL; From 5fbf39399c0df3e068a34f9e382b60a642e66f0a Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:45:04 +0200 Subject: [PATCH 6/7] lxcfs: don't cause a uaf Fixes: Coverity 355711. Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxcfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxcfs.c b/src/lxcfs.c index 6414235..ed0592a 100644 --- a/src/lxcfs.c +++ b/src/lxcfs.c @@ -1124,8 +1124,8 @@ int main(int argc, char *argv[]) } else if (strcmp(token, "nonempty") == 0) { nonempty = true; } else { - free(v); lxcfs_error("Warning: unexpected fuse option %s", v); + free(v); exit(EXIT_FAILURE); } } From 39d974839ce38dbf2c79abfe8db53a52f8c77661 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 16 Apr 2020 17:58:37 +0200 Subject: [PATCH 7/7] utils: fix recv_creds() Fixes: Coverity 355704. Fixes: Coverity 355718. Fixes: Coverity 355738. Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/utils.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/utils.c b/src/utils.c index 0be997c..5064b87 100644 --- a/src/utils.c +++ b/src/utils.c @@ -212,27 +212,14 @@ bool wait_for_sock(int sock, int timeout) bool recv_creds(int sock, struct ucred *cred, char *v) { - struct msghdr msg = { 0 }; + struct msghdr msg = {}; struct iovec iov; struct cmsghdr *cmsg; - char cmsgbuf[CMSG_SPACE(sizeof(*cred))]; - char buf[1]; - int ret; + ssize_t ret; + char cmsgbuf[CMSG_SPACE(sizeof(cred))] = {}; + char buf[1] = {'1'}; int optval = 1; - *v = '1'; - - cred->pid = -1; - cred->uid = -1; - cred->gid = -1; - - if (setsockopt(sock, SOL_SOCKET, SO_PASSCRED, &optval, sizeof(optval)) == -1) - return log_error(false, "Failed to set passcred: %s\n", strerror(errno)); - - buf[0] = '1'; - if (write(sock, buf, 1) != 1) - return log_error(false, "Failed to start write on scm fd: %s\n", strerror(errno)); - msg.msg_name = NULL; msg.msg_namelen = 0; msg.msg_control = cmsgbuf; @@ -243,19 +230,29 @@ bool recv_creds(int sock, struct ucred *cred, char *v) msg.msg_iov = &iov; msg.msg_iovlen = 1; + *v = '1'; + + ret = setsockopt(sock, SOL_SOCKET, SO_PASSCRED, &optval, sizeof(optval)); + if (ret < 0) + return log_error(false, "Failed to set passcred: %s\n", strerror(errno)); + + ret = write_nointr(sock, buf, sizeof(buf)); + if (ret != 1) + return log_error(false, "Failed to start write on scm fd: %s\n", strerror(errno)); + if (!wait_for_sock(sock, 2)) return log_error(false, "Timed out waiting for scm_cred: %s\n", strerror(errno)); ret = recvmsg(sock, &msg, MSG_DONTWAIT); - if (ret < 0) + if (ret <= 0) return log_error(false, "Failed to receive scm_cred: %s\n", strerror(errno)); cmsg = CMSG_FIRSTHDR(&msg); if (cmsg && cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred)) && - cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_CREDENTIALS) { - memcpy(cred, CMSG_DATA(cmsg), sizeof(*cred)); + cmsg->cmsg_level == SOL_SOCKET && + cmsg->cmsg_type == SCM_CREDENTIALS) { + memcpy(&cred, CMSG_DATA(cmsg), sizeof(cred)); } *v = buf[0];
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel