[lxc-devel] [lxcfs/master] fixes

brauner on Github lxc-bot at linuxcontainers.org
Mon Mar 16 16:35:37 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/20200316/a6cf21af/attachment-0001.bin>
-------------- next part --------------
From 05b7a16d12d149323932f6cd889b123a32f73dac Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 16 Mar 2020 14:05:11 +0100
Subject: [PATCH 1/2] tree-wide: memory utils improvements

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/bindings.c                |  8 +--
 src/cgroups/cgfsng.c          | 15 +-----
 src/cgroups/cgroup2_devices.c |  4 +-
 src/cgroups/cgroup_utils.c    | 10 ++--
 src/lxcfs.c                   |  5 +-
 src/macro.h                   | 14 ++++++
 src/memory_utils.h            | 95 +++++++++++++++++------------------
 src/proc_fuse.c               |  2 +-
 src/proc_loadavg.c            |  2 +-
 src/utils.c                   |  6 +--
 10 files changed, 79 insertions(+), 82 deletions(-)

diff --git a/src/bindings.c b/src/bindings.c
index c297f70..115ea1f 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -259,7 +259,7 @@ static void prune_initpid_store(void)
 static void save_initpid(struct stat *sb, pid_t pid)
 {
 	__do_free struct pidns_init_store *entry = NULL;
-	__do_close_prot_errno int pidfd = -EBADF;
+	__do_close int pidfd = -EBADF;
 	char path[LXCFS_PROC_PID_LEN];
 	struct lxcfs_opts *opts = fuse_get_context()->private_data;
 	struct stat st;
@@ -371,7 +371,7 @@ static pid_t lxcfs_clone(int (*fn)(void *), void *arg, int flags)
  */
 static void write_task_init_pid_exit(int sock, pid_t target)
 {
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	char path[LXCFS_PROC_PID_NS_LEN];
 	pid_t pid;
 
@@ -529,7 +529,7 @@ static bool is_on_ramfs(void)
 
 static int pivot_enter()
 {
-	__do_close_prot_errno int oldroot = -EBADF, newroot = -EBADF;
+	__do_close int oldroot = -EBADF, newroot = -EBADF;
 
 	oldroot = open("/", O_DIRECTORY | O_RDONLY | O_CLOEXEC);
 	if (oldroot < 0)
@@ -756,7 +756,7 @@ static void sigusr2_toggle_virtualization(int signo, siginfo_t *info, void *extr
 
 static void __attribute__((constructor)) lxcfs_init(void)
 {
-	__do_close_prot_errno int init_ns = -EBADF, root_fd = -EBADF,
+	__do_close int init_ns = -EBADF, root_fd = -EBADF,
 				  pidfd = -EBADF;
 	int i = 0;
 	pid_t pid;
diff --git a/src/cgroups/cgfsng.c b/src/cgroups/cgfsng.c
index aba457b..6ae6c1c 100644
--- a/src/cgroups/cgfsng.c
+++ b/src/cgroups/cgfsng.c
@@ -46,19 +46,6 @@
 #include "cgroup2_devices.h"
 #include "cgroup_utils.h"
 
-static void free_string_list(char **clist)
-{
-	int i;
-
-	if (!clist)
-		return;
-
-	for (i = 0; clist[i]; i++)
-		free(clist[i]);
-
-	free(clist);
-}
-
 /* Given a pointer to a null-terminated array of pointers, realloc to add one
  * entry, and point the new entry to NULL. Do not fail. Return the index to the
  * second-to-last entry - that is, the one which is now available for use
@@ -656,7 +643,7 @@ static char *readat_cpuset(int cgroup_fd)
 static int cgfsng_get_cpuset_cpus(struct cgroup_ops *ops, const char *cgroup,
 				  char **value)
 {
-	__do_close_prot_errno int cgroup_fd = -EBADF;
+	__do_close int cgroup_fd = -EBADF;
 	__do_free char *path = NULL;
 	char *v;
 	struct hierarchy *h;
diff --git a/src/cgroups/cgroup2_devices.c b/src/cgroups/cgroup2_devices.c
index d558b04..ad18dfb 100644
--- a/src/cgroups/cgroup2_devices.c
+++ b/src/cgroups/cgroup2_devices.c
@@ -345,7 +345,7 @@ int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 			      const char *path, uint32_t flags)
 {
 	__do_free char *copy = NULL;
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	union bpf_attr attr;
 	int ret;
 
@@ -400,7 +400,7 @@ int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 int bpf_program_cgroup_detach(struct bpf_program *prog)
 {
 	int ret;
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 
 	if (!prog)
 		return 0;
diff --git a/src/cgroups/cgroup_utils.c b/src/cgroups/cgroup_utils.c
index 7bdf65b..09bb0e6 100644
--- a/src/cgroups/cgroup_utils.c
+++ b/src/cgroups/cgroup_utils.c
@@ -289,7 +289,7 @@ static int check_symlink(int fd)
  */
 static int open_if_safe(int dirfd, const char *nextpath)
 {
-	__do_close_prot_errno int newfd = -EBADF;
+	__do_close int newfd = -EBADF;
 
 	newfd = openat(dirfd, nextpath, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
 	if (newfd >= 0) /* Was not a symlink, all good. */
@@ -332,7 +332,7 @@ static int open_if_safe(int dirfd, const char *nextpath)
  */
 static int open_without_symlink(const char *target, const char *prefix_skip)
 {
-	__do_close_prot_errno int dirfd = -EBADF;
+	__do_close int dirfd = -EBADF;
 	__do_free char *dup = NULL;
 	int curlen = 0, fulllen, i;
 
@@ -399,7 +399,7 @@ static int open_without_symlink(const char *target, const char *prefix_skip)
 int safe_mount(const char *src, const char *dest, const char *fstype,
 	       unsigned long flags, const void *data, const char *rootfs)
 {
-	__do_close_prot_errno int destfd = -EBADF, srcfd = -EBADF;
+	__do_close int destfd = -EBADF, srcfd = -EBADF;
 	int ret;
 	/* Only needs enough for /proc/self/fd/<fd>. */
 	char srcbuf[50], destbuf[50];
@@ -464,7 +464,7 @@ size_t strlcat(char *d, const char *s, size_t n)
 
 FILE *fopen_cloexec(const char *path, const char *mode)
 {
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	__do_fclose FILE *ret = NULL;
 	int open_mode = 0;
 	int step = 0;
@@ -673,7 +673,7 @@ char *cg_legacy_get_current_cgroup(pid_t pid, const char *controller)
 
 char *readat_file(int dirfd, const char *path)
 {
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	__do_free char *line = NULL;
 	__do_fclose FILE *f = NULL;
 	char *buf = NULL;
diff --git a/src/lxcfs.c b/src/lxcfs.c
index 041cd85..4dfb6b9 100644
--- a/src/lxcfs.c
+++ b/src/lxcfs.c
@@ -1025,7 +1025,7 @@ static bool swallow_option(int *argcp, char *argv[], char *opt, char **v)
 
 static int set_pidfile(char *pidfile)
 {
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	char buf[INTTYPE_TO_STRLEN(long)];
 	int ret;
 	struct flock fl = {
@@ -1060,7 +1060,7 @@ static int set_pidfile(char *pidfile)
 
 int main(int argc, char *argv[])
 {
-	__do_close_prot_errno int pidfile_fd = -EBADF;
+	int pidfile_fd = -EBADF;
 	int ret = EXIT_FAILURE;
 	char *pidfile = NULL, *saveptr = NULL, *token = NULL, *v = NULL;
 	char pidfile_buf[STRLITERALLEN(RUNTIME_PATH) + STRLITERALLEN("/lxcfs.pid") + 1] = {};
@@ -1198,5 +1198,6 @@ int main(int argc, char *argv[])
 		dlclose(dlopen_handle);
 	if (pidfile)
 		unlink(pidfile);
+	close_prot_errno_disarm(pidfile_fd);
 	exit(ret);
 }
diff --git a/src/macro.h b/src/macro.h
index 29b5ed7..aba00bf 100644
--- a/src/macro.h
+++ b/src/macro.h
@@ -83,6 +83,20 @@
 				    ? 20          \
 				    : sizeof(int[-2 * (sizeof(type) > 8)])))
 
+#define move_ptr(ptr)                                 \
+	({                                            \
+		typeof(ptr) __internal_ptr__ = (ptr); \
+		(ptr) = NULL;                         \
+		__internal_ptr__;                     \
+	})
+
+#define move_fd(fd)                         \
+	({                                  \
+		int __internal_fd__ = (fd); \
+		(fd) = -EBADF;              \
+		__internal_fd__;            \
+	})
+
 #define ret_errno(__errno__)       \
 	({                         \
 		errno = __errno__; \
diff --git a/src/memory_utils.h b/src/memory_utils.h
index 0328d18..87d9c6d 100644
--- a/src/memory_utils.h
+++ b/src/memory_utils.h
@@ -3,42 +3,24 @@
 #ifndef __LXCFS_MEMORY_UTILS_H
 #define __LXCFS_MEMORY_UTILS_H
 
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-
-#ifndef FUSE_USE_VERSION
-#define FUSE_USE_VERSION 26
-#endif
-
-#define _FILE_OFFSET_BITS 64
-
 #include <dirent.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "config.h"
 #include "macro.h"
 
-static inline void __auto_free__(void *p)
-{
-	free(*(void **)p);
-}
-
-static inline void __auto_fclose__(FILE **f)
-{
-	if (*f)
-		fclose(*f);
-}
+#define define_cleanup_function(type, cleaner)           \
+	static inline void cleaner##_function(type *ptr) \
+	{                                                \
+		if (*ptr)                                \
+			cleaner(*ptr);                   \
+	}
 
-static inline void __auto_closedir__(DIR **d)
-{
-	if (*d)
-		closedir(*d);
-}
+#define call_cleaner(cleaner) __attribute__((__cleanup__(cleaner##_function)))
 
 #define close_prot_errno_disarm(fd) \
 	if (fd >= 0) {              \
@@ -49,12 +31,24 @@ static inline void __auto_closedir__(DIR **d)
 	}
 
 #define close_prot_errno_replace(fd, new_fd) \
-	if (fd >= 0) {                       \
-		int _e_ = errno;             \
-		close(fd);                   \
-		errno = _e_;                 \
-		fd = new_fd;                 \
-	}
+       if (fd >= 0) {                       \
+               int _e_ = errno;             \
+               close(fd);                   \
+               errno = _e_;                 \
+               fd = new_fd;                 \
+       }
+
+static inline void close_prot_errno_disarm_function(int *fd)
+{
+       close_prot_errno_disarm(*fd);
+}
+#define __do_close call_cleaner(close_prot_errno_disarm)
+
+define_cleanup_function(FILE *, fclose);
+#define __do_fclose call_cleaner(fclose)
+
+define_cleanup_function(DIR *, closedir);
+#define __do_closedir call_cleaner(closedir)
 
 #define free_disarm(ptr)       \
 	({                     \
@@ -62,29 +56,30 @@ static inline void __auto_closedir__(DIR **d)
 		move_ptr(ptr); \
 	})
 
-static inline void __auto_close__(int *fd)
+static inline void free_disarm_function(void *ptr)
 {
-	close_prot_errno_disarm(*fd);
+	free_disarm(*(void **)ptr);
 }
+#define __do_free call_cleaner(free_disarm)
 
-#define __do_close_prot_errno __attribute__((__cleanup__(__auto_close__)))
-#define __do_free __attribute__((__cleanup__(__auto_free__)))
-#define __do_fclose __attribute__((__cleanup__(__auto_fclose__)))
-#define __do_closedir __attribute__((__cleanup__(__auto_closedir__)))
+static inline void free_string_list(char **list)
+{
+	if (list) {
+		for (int i = 0; list[i]; i++)
+			free(list[i]);
+		free_disarm(list);
+	}
+}
+define_cleanup_function(char **, free_string_list);
+#define __do_free_string_list call_cleaner(free_string_list)
 
-#define move_ptr(ptr)                                 \
-	({                                            \
-		typeof(ptr) __internal_ptr__ = (ptr); \
-		(ptr) = NULL;                         \
-		__internal_ptr__;                     \
-	})
+static inline void *memdup(const void *data, size_t len)
+{
+	void *copy = NULL;
 
-#define move_fd(fd)                         \
-	({                                  \
-		int __internal_fd__ = (fd); \
-		(fd) = -EBADF;              \
-		__internal_fd__;            \
-	})
+	copy = len ? malloc(len) : NULL;
+	return copy ? memcpy(copy, data, len) : NULL;
+}
 
 #define zalloc(__size__) (calloc(1, __size__))
 
diff --git a/src/proc_fuse.c b/src/proc_fuse.c
index 728ddf0..c3870fb 100644
--- a/src/proc_fuse.c
+++ b/src/proc_fuse.c
@@ -965,7 +965,7 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
 /* Note that "memory.stat" in cgroup2 is hierarchical by default. */
 static bool cgroup_parse_memory_stat(const char *cgroup, struct memory_stat *mstat)
 {
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	__do_fclose FILE *f = NULL;
 	__do_free char *line = NULL;
 	__do_free void *fdopen_cache = NULL;
diff --git a/src/proc_loadavg.c b/src/proc_loadavg.c
index 0a4f6d6..37a8aa8 100644
--- a/src/proc_loadavg.c
+++ b/src/proc_loadavg.c
@@ -284,7 +284,7 @@ static int calc_pid(char ***pid_buf, char *dpath, int depth, int sum, int cfd)
 {
 	__do_free char *path = NULL;
 	__do_free void *fdopen_cache = NULL;
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 	__do_fclose FILE *f = NULL;
 	__do_closedir DIR *dir = NULL;
 	struct dirent *file;
diff --git a/src/utils.c b/src/utils.c
index 5b5e076..e54f202 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -87,7 +87,7 @@ char *must_strcat(char **src, size_t *sz, size_t *asz, const char *format, ...)
  */
 static int in_same_namespace(pid_t pid1, pid_t pid2, const char *ns)
 {
-	__do_close_prot_errno int ns_fd1 = -1, ns_fd2 = -1;
+	__do_close int ns_fd1 = -1, ns_fd2 = -1;
 	int ret = -1;
 	struct stat ns_st1, ns_st2;
 
@@ -176,7 +176,7 @@ void do_release_file_info(struct fuse_file_info *fi)
 
 bool wait_for_sock(int sock, int timeout)
 {
-	__do_close_prot_errno int epfd = -EBADF;
+	__do_close int epfd = -EBADF;
 	struct epoll_event ev;
 	int ret, now, starttime, deltatime;
 
@@ -463,7 +463,7 @@ static char *fd_to_buf(int fd, size_t *length)
 
 static char *file_to_buf(const char *path, size_t *length)
 {
-	__do_close_prot_errno int fd = -EBADF;
+	__do_close int fd = -EBADF;
 
 	if (!length)
 		return NULL;

From e7f9baee1dc6fe318b7ad5159dff90c0759ea7ab Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 16 Mar 2020 17:34:34 +0100
Subject: [PATCH 2/2] tree-wide: fix dot_or_empty()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/cgroups/cgfsng.c       | 29 ++++++++++++++++++++++++-----
 src/cgroups/cgroup_utils.h |  5 +++++
 src/proc_cpuview.c         |  6 +++++-
 src/proc_loadavg.c         |  2 +-
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/src/cgroups/cgfsng.c b/src/cgroups/cgfsng.c
index 6ae6c1c..61916b8 100644
--- a/src/cgroups/cgfsng.c
+++ b/src/cgroups/cgfsng.c
@@ -543,7 +543,11 @@ static bool cgfsng_get(struct cgroup_ops *ops, const char *controller,
 	if (!h)
 		return false;
 
-	path = must_make_path(dot_or_empty(cgroup), cgroup, file, NULL);
+	if (is_relative(cgroup))
+		path = must_make_path(cgroup, file, NULL);
+	else
+		path = must_make_path(".", cgroup, file, NULL);
+
 	*value = readat_file(h->fd, path);
 	return *value != NULL;
 }
@@ -573,7 +577,11 @@ static int cgfsng_get_memory(struct cgroup_ops *ops, const char *cgroup,
 		ret = CGROUP2_SUPER_MAGIC;
 	}
 
-	path = must_make_path(dot_or_empty(cgroup), cgroup, file, NULL);
+	if (is_relative(cgroup))
+		path = must_make_path(cgroup, file, NULL);
+	else
+		path = must_make_path(".", cgroup, file, NULL);
+
 	*value = readat_file(h->fd, path);
 	if (!*value)
 		ret = -1;
@@ -590,7 +598,11 @@ static int cgfsng_get_memory_stats_fd(struct cgroup_ops *ops, const char *cgroup
 	if (!h)
 		return -1;
 
-	path = must_make_path(dot_or_empty(cgroup), cgroup, "memory.stat", NULL);
+	if (is_relative(cgroup))
+		path = must_make_path(cgroup, "memory.stat", NULL);
+	else
+		path = must_make_path(".", cgroup, "memory.stat", NULL);
+
 	return openat(h->fd, path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
 }
 
@@ -659,7 +671,10 @@ static int cgfsng_get_cpuset_cpus(struct cgroup_ops *ops, const char *cgroup,
 		ret = CGROUP2_SUPER_MAGIC;
 
 	*value = NULL;
-	path = must_make_path(dot_or_empty(cgroup), cgroup, NULL);
+	if (is_relative(cgroup))
+		path = must_make_path(cgroup, NULL);
+	else
+		path = must_make_path(".", cgroup, NULL);
 	cgroup_fd = openat_safe(h->fd, path);
 	if (cgroup_fd < 0) {
 		return -1;
@@ -709,7 +724,11 @@ static int cgfsng_get_io(struct cgroup_ops *ops, const char *cgroup,
 	else
 		ret = CGROUP2_SUPER_MAGIC;
 
-	path = must_make_path(dot_or_empty(cgroup), cgroup, file, NULL);
+	if (is_relative(cgroup))
+		path = must_make_path(cgroup, file, NULL);
+	else
+		path = must_make_path(".", cgroup, file, NULL);
+
 	*value = readat_file(h->fd, path);
 	if (!*value) {
 		if (errno == ENOENT)
diff --git a/src/cgroups/cgroup_utils.h b/src/cgroups/cgroup_utils.h
index 483ef49..35e5a98 100644
--- a/src/cgroups/cgroup_utils.h
+++ b/src/cgroups/cgroup_utils.h
@@ -102,4 +102,9 @@ static inline const char *dot_or_empty(const char *s)
 	return (*s == '/') ? dot : empty;
 }
 
+static inline bool is_relative(const char *s)
+{
+	return s && *s != '/';
+}
+
 #endif /* __LXC_CGROUP_UTILS_H */
diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index ed6bd77..a4e59e6 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -243,7 +243,11 @@ static bool cgfs_param_exist(const char *controller, const char *cgroup,
 	if (cfd < 0)
 		return false;
 
-	path = must_make_path(dot_or_empty(cgroup), cgroup, file, NULL);
+	if (is_relative(cgroup))
+		path = must_make_path(cgroup, file, NULL);
+	else
+		path = must_make_path(".", cgroup, file, NULL);
+
 	return (faccessat(cfd, path, F_OK, 0) == 0);
 }
 
diff --git a/src/proc_loadavg.c b/src/proc_loadavg.c
index 37a8aa8..20b538e 100644
--- a/src/proc_loadavg.c
+++ b/src/proc_loadavg.c
@@ -523,7 +523,7 @@ static void *load_begin(void *arg)
 				if  (!path)
 					goto out;
 
-				ret = snprintf(path, length, "%s%s", dot_or_empty(f->cg), f->cg);
+				ret = snprintf(path, length, "%s%s", !is_relative(f->cg) ? "." : "", f->cg);
 				/* Ignore the node if snprintf fails.*/
 				if (ret < 0 || ret > length - 1)
 					log_error(goto out, "Refresh node %s failed for snprintf()", f->cg);


More information about the lxc-devel mailing list