[lxc-devel] [lxcfs/master] fixes
brauner on Github
lxc-bot at linuxcontainers.org
Thu Apr 16 16:00:39 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 365 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200416/0b3869fc/attachment.bin>
-------------- next part --------------
From bea116fc4c685436323370c75311661b7d1ce81d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.bumiller at proxmox.com>
Signed-off-by: Christian Brauner <christian.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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];
More information about the lxc-devel
mailing list