[lxc-devel] [lxcfs/master] support ASAN and UBSAN + bugfixes

brauner on Github lxc-bot at linuxcontainers.org
Thu Feb 27 21:30:49 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/20200227/b04ff7b8/attachment.bin>
-------------- next part --------------
From 8d49f03bc6bdf01d16f90f55ebcc3eb316e6ad94 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 20:38:22 +0100
Subject: [PATCH 1/3] cgroup_utils: re-add O_NOFOLLOW

This got removed on accident.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 cgroups/cgroup_utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cgroups/cgroup_utils.c b/cgroups/cgroup_utils.c
index 896ed43..805ee32 100644
--- a/cgroups/cgroup_utils.c
+++ b/cgroups/cgroup_utils.c
@@ -291,12 +291,12 @@ static int open_if_safe(int dirfd, const char *nextpath)
 {
 	__do_close_prot_errno int newfd = -EBADF;
 
-	newfd = openat(dirfd, nextpath, O_RDONLY | O_CLOEXEC);
+	newfd = openat(dirfd, nextpath, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
 	if (newfd >= 0) /* Was not a symlink, all good. */
 		return move_fd(newfd);
 
 	if (errno == ELOOP)
-		return move_fd(newfd);
+		return -1;
 
 	if (errno == EPERM || errno == EACCES) {
 		/* We're not root (cause we got EPERM) so try opening with

From 7eac38257f55d4ad92ebef350b4333879e28bfa4 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 21:48:39 +0100
Subject: [PATCH 2/3] autotools: add support for ASAN and UBSAN

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 Makefile.am  | 20 ++++++++++++++++++++
 configure.ac | 25 +++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 5029ef3..e6dc703 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -28,6 +28,16 @@ liblxcfs_la_SOURCES = api_extension.h \
 		      sysfs_fuse.c sysfs_fuse.h \
 		      utils.c utils.h
 liblxcfs_la_CFLAGS = $(AM_CFLAGS)
+
+if ENABLE_ASAN
+liblxcfs_la_CFLAGS += -fsanitize=address \
+		      -fno-omit-frame-pointer
+endif
+
+if ENABLE_UBSAN
+liblxcfs_la_CFLAGS += -fsanitize=undefined
+endif
+
 liblxcfs_la_LDFLAGS = $(AM_CFLAGS) -module -avoid-version -shared
 
 liblxcfstest_la_SOURCES = api_extension.h \
@@ -46,6 +56,16 @@ liblxcfstest_la_SOURCES = api_extension.h \
 			  sysfs_fuse.c sysfs_fuse.h \
 			  utils.c utils.h
 liblxcfstest_la_CFLAGS = $(AM_CFLAGS) -DRELOADTEST
+
+if ENABLE_ASAN
+liblxcfstest_la_CFLAGS += -fsanitize=address \
+			  -fno-omit-frame-pointer
+endif
+
+if ENABLE_UBSAN
+liblxcfstest_la_CFLAGS += -fsanitize=undefined
+endif
+
 liblxcfstest_la_LDFLAGS = $(AM_CFLAGS) -module -avoid-version -shared
 
 noinst_HEADERS = bindings.h \
diff --git a/configure.ac b/configure.ac
index ad2b0b8..dbe26e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -230,4 +230,29 @@ AX_CHECK_COMPILE_FLAG([-fexceptions], [CFLAGS="$CFLAGS -fexceptions"],,[-Werror]
 AX_CHECK_LINK_FLAG([-z relro], [LDFLAGS="$LDFLAGS -z relro"],,[])
 AX_CHECK_LINK_FLAG([-z now], [LDFLAGS="$LDFLAGS -z now"],,[])
 
+# Build with ASAN commands
+AC_ARG_ENABLE([asan],
+	[AS_HELP_STRING([--enable-asan], [build with address sanitizer enabled [default=no]])],
+	[enable_asan=$enableval], [enable_asan=no])
+AM_CONDITIONAL([ENABLE_ASAN], [test "x$enable_asan" = "xyes"])
+
+# Build with UBSAN commands
+AC_ARG_ENABLE([ubsan],
+	[AS_HELP_STRING([--enable-ubsan], [build with ubsan sanitizer enabled [default=no]])],
+	[enable_asan=$enableval], [enable_ubsan=no])
+AM_CONDITIONAL([ENABLE_UBSAN], [test "x$enable_ubsan" = "xyes"])
+
 AC_OUTPUT
+
+# Configuration overview
+cat << EOF
+
+----------------------------
+Environment:
+ - compiler: $CC
+
+Debugging:
+ - tests: $enable_tests
+ - ASAN: $enable_asan
+ - mutex debugging: $enable_mutex_debugging
+EOF

From 700dd417de46bf96d9abe7b4beb506ecc8af61db Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 22:23:45 +0100
Subject: [PATCH 3/3] tree-wide: fix memory leaks

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 proc_cpuview.c |  6 ++----
 proc_fuse.c    |  4 ++--
 sysfs_fuse.c   | 44 +++++++++++++++++++++++++-------------------
 utils.c        |  2 +-
 4 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/proc_cpuview.c b/proc_cpuview.c
index 5d29a5c..9551987 100644
--- a/proc_cpuview.c
+++ b/proc_cpuview.c
@@ -482,10 +482,10 @@ static double exact_cpu_count(const char *cg)
  */
 int max_cpu_count(const char *cg)
 {
+	__do_free char *cpuset = NULL;
 	int rv, nprocs;
 	int64_t cfs_quota, cfs_period;
 	int nr_cpus_in_cpuset = 0;
-	char *cpuset = NULL;
 
 	if (!read_cpu_cfs_param(cg, "quota", &cfs_quota))
 		return 0;
@@ -616,16 +616,14 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 		max_cpus = cpu_cnt;
 
 	stat_node = find_or_create_proc_stat_node(cg_cpu_usage, nprocs, cg);
-
 	if (!stat_node) {
 		lxcfs_error("unable to find/create stat node for %s\n", cg);
 		return 0;
 	}
 
 	diff = malloc(sizeof(struct cpuacct_usage) * nprocs);
-	if (!diff) {
+	if (!diff)
 		return 0;
-	}
 
 	/*
 	 * If the new values are LOWER than values stored in memory, it means
diff --git a/proc_fuse.c b/proc_fuse.c
index 6a14e90..afe11a2 100644
--- a/proc_fuse.c
+++ b/proc_fuse.c
@@ -138,8 +138,8 @@ static off_t get_procfile_size(const char *which)
 
 int proc_open(const char *path, struct fuse_file_info *fi)
 {
+	__do_free struct file_info *info = NULL;
 	int type = -1;
-	struct file_info *info;
 
 	if (strcmp(path, "/proc/meminfo") == 0)
 		type = LXC_TYPE_PROC_MEMINFO;
@@ -175,7 +175,7 @@ int proc_open(const char *path, struct fuse_file_info *fi)
 	/* set actual size to buffer size */
 	info->size = info->buflen;
 
-	fi->fh = PTR_TO_UINT64(info);
+	fi->fh = PTR_TO_UINT64(move_ptr(info));
 	return 0;
 }
 
diff --git a/sysfs_fuse.c b/sysfs_fuse.c
index 599790d..988cc03 100644
--- a/sysfs_fuse.c
+++ b/sysfs_fuse.c
@@ -39,6 +39,7 @@
 #include <sys/vfs.h>
 
 #include "bindings.h"
+#include "memory_utils.h"
 #include "cgroups/cgroup.h"
 #include "config.h"
 #include "sysfs_fuse.h"
@@ -48,11 +49,10 @@ static int sys_devices_system_cpu_online_read(char *buf, size_t size,
 					      off_t offset,
 					      struct fuse_file_info *fi)
 {
+	__do_free char *cg = NULL, *cpuset = NULL;
 	struct fuse_context *fc = fuse_get_context();
 	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	char *cache = d->buf;
-	char *cg;
-	char *cpuset = NULL;
 	bool use_view;
 
 	int max_cpus = 0;
@@ -60,13 +60,18 @@ static int sys_devices_system_cpu_online_read(char *buf, size_t size,
 	ssize_t total_len = 0;
 
 	if (offset) {
+		int left;
+
 		if (!d->cached)
 			return 0;
+
 		if (offset > d->size)
 			return -EINVAL;
-		int left = d->size - offset;
+
+		left = d->size - offset;
 		total_len = left > size ? size : left;
 		memcpy(buf, cache + offset, total_len);
+
 		return total_len;
 	}
 
@@ -80,7 +85,7 @@ static int sys_devices_system_cpu_online_read(char *buf, size_t size,
 
 	cpuset = get_cpuset(cg);
 	if (!cpuset)
-		goto err;
+		return 0;
 
 	use_view = cgroup_ops->can_use_cpuview(cgroup_ops);
 	if (use_view)
@@ -104,27 +109,23 @@ static int sys_devices_system_cpu_online_read(char *buf, size_t size,
 		total_len = size;
 
 	memcpy(buf, d->buf, total_len);
-err:
-	free(cpuset);
-	free(cg);
+
 	return total_len;
 }
 
 static off_t get_sysfile_size(const char *which)
 {
-	FILE *f;
-	char *line = NULL;
+	__do_fclose FILE *f = NULL;
+	__do_free char *line = NULL;
 	size_t len = 0;
 	ssize_t sz, answer = 0;
 
-	f = fopen(which, "r");
+	f = fopen(which, "re");
 	if (!f)
 		return 0;
 
 	while ((sz = getline(&line, &len, f)) != -1)
 		answer += sz;
-	fclose(f);
-	free(line);
 
 	return answer;
 }
@@ -205,8 +206,8 @@ int sys_readdir(const char *path, void *buf, fuse_fill_dir_t filler,
 
 int sys_open(const char *path, struct fuse_file_info *fi)
 {
+	__do_free struct file_info *info = NULL;
 	int type = -1;
-	struct file_info *info;
 
 	if (strcmp(path, "/sys/devices") == 0)
 		type = LXC_TYPE_SYS_DEVICES;
@@ -227,14 +228,16 @@ int sys_open(const char *path, struct fuse_file_info *fi)
 	info->type = type;
 
 	info->buflen = get_sysfile_size(path) + BUF_RESERVE_SIZE;
-	do {
-		info->buf = malloc(info->buflen);
-	} while (!info->buf);
+
+	info->buf = malloc(info->buflen);
+	if (!info->buf)
+		return -ENOMEM;
+
 	memset(info->buf, 0, info->buflen);
 	/* set actual size to buffer size */
 	info->size = info->buflen;
 
-	fi->fh = PTR_TO_UINT64(info);
+	fi->fh = PTR_TO_UINT64(move_ptr(info));
 	return 0;
 }
 
@@ -276,9 +279,12 @@ int sys_read(const char *path, char *buf, size_t size, off_t offset,
 	case LXC_TYPE_SYS_DEVICES_SYSTEM_CPU_ONLINE:
 		return sys_devices_system_cpu_online_read(buf, size, offset, fi);
 	case LXC_TYPE_SYS_DEVICES:
+		break;
 	case LXC_TYPE_SYS_DEVICES_SYSTEM:
+		break;
 	case LXC_TYPE_SYS_DEVICES_SYSTEM_CPU:
-	default:
-		return -EINVAL;
+		break;
 	}
+
+	return -EINVAL;
 }
diff --git a/utils.c b/utils.c
index 50c940f..e49bc4f 100644
--- a/utils.c
+++ b/utils.c
@@ -380,7 +380,7 @@ void prune_init_slice(char *cg)
 	point = cg + cg_len - initscope_len;
 	if (strcmp(point, INITSCOPE) == 0) {
 		if (point == cg)
-			*(point+1) = '\0';
+			*(point + 1) = '\0';
 		else
 			*point = '\0';
 	}


More information about the lxc-devel mailing list