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

Reply via email to