Merge pull request #3317 from brauner/2020-03-20/fixes

src/lxc/cgroups/cgfsng.c

@@ -895,13 +895,15 @@ static int get_existing_subsystems(char ***klist, char ***nlist)
 	return 0;
 }
 
-static void trim(char *s)
+static char *trim(char *s)
 {
 	size_t len;
 
 	len = strlen(s);
 	while ((len > 1) && (s[len - 1] == '\n'))
 		s[--len] = '\0';
+
+	return s;
 }
 
 static void lxc_cgfsng_print_hierarchies(struct cgroup_ops *ops)
@@ -1683,8 +1685,8 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 				      const char *root, int type)
 {
 	__do_free char *cgroup_root = NULL;
+	bool has_cgns = false, wants_force_mount = false;
 	int ret;
-	bool has_cgns = false, retval = false, wants_force_mount = false;
 
 	if (!ops)
 		return ret_set_errno(false, ENOENT);
@@ -1722,12 +1724,12 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 	cgroup_root = must_make_path(root, DEFAULT_CGROUP_MOUNTPOINT, NULL);
 	if (ops->cgroup_layout == CGROUP_LAYOUT_UNIFIED) {
 		if (has_cgns && wants_force_mount) {
-			/* If cgroup namespaces are supported but the container
+			/*
+			 * If cgroup namespaces are supported but the container
 			 * will not have CAP_SYS_ADMIN after it has started we
 			 * need to mount the cgroups manually.
 			 */
-			return cg_mount_in_cgroup_namespace(type, ops->unified,
-							    cgroup_root) == 0;
+			return cg_mount_in_cgroup_namespace(type, ops->unified, cgroup_root) == 0;
 		}
 
 		return cg_mount_cgroup_full(type, ops->unified, cgroup_root) == 0;
@@ -1738,7 +1740,7 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 			 MS_NOSUID | MS_NODEV | MS_NOEXEC | MS_RELATIME,
 			 "size=10240k,mode=755", root);
 	if (ret < 0)
-		goto on_error;
+		return false;
 
 	for (int i = 0; ops->hierarchies[i]; i++) {
 		__do_free char *controllerpath = NULL, *path2 = NULL;
@@ -1754,10 +1756,8 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 			continue;
 
 		ret = mkdir(controllerpath, 0755);
-		if (ret < 0) {
-			ERROR("Error creating cgroup path: %s", controllerpath);
-			goto on_error;
-		}
+		if (ret < 0)
+			return log_error_errno(false, errno, "Error creating cgroup path: %s", controllerpath);
 
 		if (has_cgns && wants_force_mount) {
 			/* If cgroup namespaces are supported but the container
@@ -1766,14 +1766,14 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 			 */
 			ret = cg_mount_in_cgroup_namespace(type, h, controllerpath);
 			if (ret < 0)
-				goto on_error;
+				return false;
 
 			continue;
 		}
 
 		ret = cg_mount_cgroup_full(type, h, controllerpath);
 		if (ret < 0)
-			goto on_error;
+			return false;
 
 		if (!cg_mount_needs_subdirs(type))
 			continue;
@@ -1782,17 +1782,15 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 				       ops->container_cgroup, NULL);
 		ret = mkdir_p(path2, 0755);
 		if (ret < 0)
-			goto on_error;
+			return false;
 
 		ret = cg_legacy_mount_controllers(type, h, controllerpath,
 						  path2, ops->container_cgroup);
 		if (ret < 0)
-			goto on_error;
+			return false;
 	}
-	retval = true;
 
-on_error:
-	return retval;
+	return true;
 }
 
 /* Only root needs to escape to the cgroup of its init. */
@@ -1913,7 +1911,7 @@ static int freezer_cgroup_events_cb(int fd, uint32_t events, void *cbdata,
 static int cg_unified_freeze(struct cgroup_ops *ops, int timeout)
 {
 	__do_close int fd = -EBADF;
-	__do_lxc_mainloop_close struct lxc_epoll_descr *descr_ptr = NULL;
+	call_cleaner(lxc_mainloop_close) struct lxc_epoll_descr *descr_ptr = NULL;
 	int ret;
 	struct lxc_epoll_descr descr;
 	struct hierarchy *h;
@@ -1981,7 +1979,7 @@ static int cg_legacy_unfreeze(struct cgroup_ops *ops)
 static int cg_unified_unfreeze(struct cgroup_ops *ops, int timeout)
 {
 	__do_close int fd = -EBADF;
-	__do_lxc_mainloop_close struct lxc_epoll_descr *descr_ptr = NULL;
+	call_cleaner(lxc_mainloop_close)struct lxc_epoll_descr *descr_ptr = NULL;
 	int ret;
 	struct lxc_epoll_descr descr;
 	struct hierarchy *h;
@@ -3001,8 +2999,8 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg
 static char *cg_unified_get_current_cgroup(bool relative)
 {
 	__do_free char *basecginfo = NULL;
+	char *copy;
 	char *base_cgroup;
-	char *copy = NULL;
 
 	if (!relative && (geteuid() == 0))
 		basecginfo = read_file("/proc/1/cgroup");
@@ -3013,18 +3011,14 @@ static char *cg_unified_get_current_cgroup(bool relative)
 
 	base_cgroup = strstr(basecginfo, "0::/");
 	if (!base_cgroup)
-		goto cleanup_on_err;
+		return NULL;
 
 	base_cgroup = base_cgroup + 3;
 	copy = copy_to_eol(base_cgroup);
 	if (!copy)
-		goto cleanup_on_err;
-
-cleanup_on_err:
-	if (copy)
-		trim(copy);
+		return NULL;
 
-	return copy;
+	return trim(copy);
 }
 
 static int cg_unified_init(struct cgroup_ops *ops, bool relative,

src/lxc/mainloop.c

@@ -34,7 +34,7 @@ int lxc_mainloop(struct lxc_epoll_descr *descr, int timeout_ms)
 			if (errno == EINTR)
 				continue;
 
-			return -1;
+			return -errno;
 		}
 
 		for (i = 0; i < nfds; i++) {
@@ -62,9 +62,9 @@ int lxc_mainloop(struct lxc_epoll_descr *descr, int timeout_ms)
 int lxc_mainloop_add_handler(struct lxc_epoll_descr *descr, int fd,
 			     lxc_mainloop_callback_t callback, void *data)
 {
+	__do_free struct mainloop_handler *handler = NULL;
+	__do_free struct lxc_list *item = NULL;
 	struct epoll_event ev;
-	struct mainloop_handler *handler;
-	struct lxc_list *item;
 
 	if (fd < 0)
 		return -1;
@@ -81,19 +81,15 @@ int lxc_mainloop_add_handler(struct lxc_epoll_descr *descr, int fd,
 	ev.data.ptr = handler;
 
 	if (epoll_ctl(descr->epfd, EPOLL_CTL_ADD, fd, &ev) < 0)
-		goto out_free_handler;
+		return -errno;
 
 	item = malloc(sizeof(*item));
 	if (!item)
-		goto out_free_handler;
+		return ret_errno(ENOMEM);
 
-	item->elem = handler;
-	lxc_list_add(&descr->handlers, item);
+	item->elem = move_ptr(handler);
+	lxc_list_add(&descr->handlers, move_ptr(item));
 	return 0;
-
-out_free_handler:
-	free(handler);
-	return -1;
 }
 
 int lxc_mainloop_del_handler(struct lxc_epoll_descr *descr, int fd)
@@ -107,7 +103,7 @@ int lxc_mainloop_del_handler(struct lxc_epoll_descr *descr, int fd)
 		if (handler->fd == fd) {
 			/* found */
 			if (epoll_ctl(descr->epfd, EPOLL_CTL_DEL, fd, NULL))
-				return -1;
+				return -errno;
 
 			lxc_list_del(iterator);
 			free(iterator->elem);
@@ -116,21 +112,20 @@ int lxc_mainloop_del_handler(struct lxc_epoll_descr *descr, int fd)
 		}
 	}
 
-	return -1;
+	return ret_errno(EINVAL);
 }
 
 int lxc_mainloop_open(struct lxc_epoll_descr *descr)
 {
-	/* hint value passed to epoll create */
 	descr->epfd = epoll_create1(EPOLL_CLOEXEC);
 	if (descr->epfd < 0)
-		return -1;
+		return -errno;
 
 	lxc_list_init(&descr->handlers);
 	return 0;
 }
 
-int lxc_mainloop_close(struct lxc_epoll_descr *descr)
+void lxc_mainloop_close(struct lxc_epoll_descr *descr)
 {
 	struct lxc_list *iterator, *next;
 
@@ -144,8 +139,5 @@ int lxc_mainloop_close(struct lxc_epoll_descr *descr)
 		iterator = next;
 	}
 
-	if (descr->epfd >= 0)
-		return close(descr->epfd);
-
-	return 0;
+	close_prot_errno_disarm(descr->epfd);
 }

src/lxc/mainloop.h

@@ -6,6 +6,7 @@
 #include <stdint.h>
 
 #include "list.h"
+#include "memory_utils.h"
 
 #define LXC_MAINLOOP_ERROR -1
 #define LXC_MAINLOOP_CONTINUE 0
@@ -29,15 +30,8 @@ extern int lxc_mainloop_del_handler(struct lxc_epoll_descr *descr, int fd);
 
 extern int lxc_mainloop_open(struct lxc_epoll_descr *descr);
 
-extern int lxc_mainloop_close(struct lxc_epoll_descr *descr);
+extern void lxc_mainloop_close(struct lxc_epoll_descr *descr);
 
-static inline void __auto_lxc_mainloop_close__(struct lxc_epoll_descr **descr)
-{
-	if (*descr)
-		lxc_mainloop_close(*descr);
-}
-
-#define __do_lxc_mainloop_close \
-	__attribute__((__cleanup__(__auto_lxc_mainloop_close__)))
+define_cleanup_function(struct lxc_epoll_descr *, lxc_mainloop_close);
 
 #endif

src/lxc/ringbuf.c

@@ -20,9 +20,9 @@
 
 int lxc_ringbuf_create(struct lxc_ringbuf *buf, size_t size)
 {
+	__do_close int memfd = -EBADF;
 	char *tmp;
 	int ret;
-	int memfd = -1;
 
 	buf->size = size;
 	buf->r_off = 0;
@@ -63,14 +63,10 @@ int lxc_ringbuf_create(struct lxc_ringbuf *buf, size_t size)
 	if (tmp == MAP_FAILED || tmp != (buf->addr + buf->size))
 		goto on_error;
 
-	close(memfd);
-
 	return 0;
 
 on_error:
 	lxc_ringbuf_release(buf);
-	if (memfd >= 0)
-		close(memfd);
 	return -1;
 }
 

src/lxc/ringbuf.h

@@ -36,7 +36,8 @@ extern int lxc_ringbuf_read(struct lxc_ringbuf *buf, char *out, size_t *len);
 
 static inline void lxc_ringbuf_release(struct lxc_ringbuf *buf)
 {
-	munmap(buf->addr, buf->size * 2);
+	if (buf->addr)
+		munmap(buf->addr, buf->size * 2);
 }
 
 static inline void lxc_ringbuf_clear(struct lxc_ringbuf *buf)