[lxc-devel] [lxcfs/master] bugfixes

brauner on Github lxc-bot at linuxcontainers.org
Thu Feb 27 12:07:59 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/b23212e0/attachment.bin>
-------------- next part --------------
From c55c8774a11bd53eb424aba44b2c157188531398 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 12:10:57 +0100
Subject: [PATCH 1/4] croups: remove unused variable

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

diff --git a/cgroups/cgroup_utils.c b/cgroups/cgroup_utils.c
index 936a156..9e63578 100644
--- a/cgroups/cgroup_utils.c
+++ b/cgroups/cgroup_utils.c
@@ -469,7 +469,6 @@ FILE *fopen_cloexec(const char *path, const char *mode)
 	__do_fclose FILE *ret = NULL;
 	int open_mode = 0;
 	int step = 0;
-	int saved_errno = 0;
 
 	if (!strncmp(mode, "r+", 2)) {
 		open_mode = O_RDWR;

From 99b183fb2ff4ed2181f84e6436cec4dfa22b5be0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 12:21:43 +0100
Subject: [PATCH 2/4] tree-wide: introduce casting helpers

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 bindings.h     |  4 ++--
 cgroup_fuse.c  | 10 +++++-----
 macro.h        |  3 +++
 proc_cpuview.c |  2 +-
 proc_fuse.c    | 20 ++++++++++----------
 proc_loadavg.c |  2 +-
 sysfs_fuse.c   |  6 +++---
 utils.c        |  3 ++-
 8 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/bindings.h b/bindings.h
index 777fcd2..d995518 100644
--- a/bindings.h
+++ b/bindings.h
@@ -57,9 +57,9 @@ struct file_info {
 	char *cgroup;
 	char *file;
 	int type;
-	char *buf;  // unused as of yet
+	char *buf; /* unused */
 	int buflen;
-	int size; //actual data size
+	int size; /*actual data size */
 	int cached;
 };
 
diff --git a/cgroup_fuse.c b/cgroup_fuse.c
index 9f9325f..2ff2ea6 100644
--- a/cgroup_fuse.c
+++ b/cgroup_fuse.c
@@ -1150,7 +1150,7 @@ int cg_open(const char *path, struct fuse_file_info *fi)
 	file_info->buf = NULL;
 	file_info->buflen = 0;
 
-	fi->fh = (unsigned long)file_info;
+	fi->fh = PTR_TO_UINT64(file_info);
 	ret = 0;
 
 out:
@@ -1358,7 +1358,7 @@ int cg_read(const char *path, char *buf, size_t size, off_t offset,
 	    struct fuse_file_info *fi)
 {
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *f = (struct file_info *)fi->fh;
+	struct file_info *f = INTTYPE_TO_PTR(fi->fh);
 	struct cgfs_files *k = NULL;
 	char *data = NULL;
 	int ret, s;
@@ -1468,7 +1468,7 @@ int cg_opendir(const char *path, struct fuse_file_info *fi)
 	dir_info->file = NULL;
 	dir_info->buflen = 0;
 
-	fi->fh = (unsigned long)dir_info;
+	fi->fh = PTR_TO_UINT64(dir_info);
 	return 0;
 }
 
@@ -1811,7 +1811,7 @@ int cg_write(const char *path, const char *buf, size_t size, off_t offset,
 	struct fuse_context *fc = fuse_get_context();
 	char *localbuf = NULL;
 	struct cgfs_files *k = NULL;
-	struct file_info *f = (struct file_info *)fi->fh;
+	struct file_info *f = INTTYPE_TO_PTR(fi->fh);
 	bool r;
 
 	if (!fc || !cgroup_ops || pure_unified_layout(cgroup_ops))
@@ -1977,7 +1977,7 @@ static void free_keys(struct cgfs_files **keys)
 int cg_readdir(const char *path, void *buf, fuse_fill_dir_t filler,
 	       off_t offset, struct fuse_file_info *fi)
 {
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	struct cgfs_files **list = NULL;
 	int i, ret;
 	char *nextcg = NULL;
diff --git a/macro.h b/macro.h
index dd66f24..b4277cb 100644
--- a/macro.h
+++ b/macro.h
@@ -95,4 +95,7 @@
 		_exit(EXIT_FAILURE);                    \
 	})
 
+#define PTR_TO_UINT64(p) ((uint64_t)((intptr_t)(p)))
+#define INTTYPE_TO_PTR(u) ((void *)((intptr_t)(u)))
+
 #endif /* __LXCFS_MACRO_H */
diff --git a/proc_cpuview.c b/proc_cpuview.c
index e31d457..330438a 100644
--- a/proc_cpuview.c
+++ b/proc_cpuview.c
@@ -881,7 +881,7 @@ int proc_cpuinfo_read(char *buf, size_t size, off_t offset,
 	__do_free char *cg = NULL, *cpuset = NULL, *line = NULL;
 	__do_fclose FILE *f = NULL;
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	size_t linelen = 0, total_len = 0;
 	bool am_printing = false, firstline = true, is_s390x = false;
 	int curcpu = -1, cpu, max_cpus = 0;
diff --git a/proc_fuse.c b/proc_fuse.c
index 7717c66..0117fee 100644
--- a/proc_fuse.c
+++ b/proc_fuse.c
@@ -153,7 +153,7 @@ int proc_open(const char *path, struct fuse_file_info *fi)
 	/* set actual size to buffer size */
 	info->size = info->buflen;
 
-	fi->fh = (unsigned long)info;
+	fi->fh = PTR_TO_UINT64(info);
 	return 0;
 }
 
@@ -224,7 +224,7 @@ static int proc_swaps_read(char *buf, size_t size, off_t offset,
 	__do_free char *cg = NULL, *memswlimit_str = NULL, *memusage_str = NULL,
 		       *memswusage_str = NULL;
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	unsigned long memswlimit = 0, memlimit = 0, memusage = 0,
 		      memswusage = 0, swap_total = 0, swap_free = 0;
 	ssize_t total_len = 0;
@@ -347,7 +347,7 @@ static int proc_diskstats_read(char *buf, size_t size, off_t offset,
 		       *line = NULL;
 	__do_fclose FILE *f = NULL;
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	unsigned long read = 0, write = 0;
 	unsigned long read_merged = 0, write_merged = 0;
 	unsigned long read_sectors = 0, write_sectors = 0;
@@ -655,7 +655,7 @@ static int proc_uptime_read(char *buf, size_t size, off_t offset,
 			    struct fuse_file_info *fi)
 {
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	double busytime = get_reaper_busy(fc->pid);
 	char *cache = d->buf;
 	ssize_t total_len = 0;
@@ -707,7 +707,7 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
 	__do_free struct cpuacct_usage *cg_cpu_usage = NULL;
 	__do_fclose FILE *f = NULL;
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	size_t linelen = 0, total_len = 0;
 	int curcpu = -1; /* cpu numbering starts at 0 */
 	int physcpu = 0;
@@ -1004,7 +1004,7 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset,
 	__do_fclose FILE *f = NULL;
 	struct fuse_context *fc = fuse_get_context();
 	struct lxcfs_opts *opts = (struct lxcfs_opts *) fuse_get_context()->private_data;
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	unsigned long memlimit = 0, memusage = 0, memswlimit = 0,
 		      memswusage = 0, cached = 0, hosttotal = 0, active_anon = 0,
 		      inactive_anon = 0, active_file = 0, inactive_file = 0,
@@ -1193,9 +1193,9 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset,
 }
 
 int proc_read(const char *path, char *buf, size_t size, off_t offset,
-		struct fuse_file_info *fi)
+	      struct fuse_file_info *fi)
 {
-	struct file_info *f = (struct file_info *) fi->fh;
+	struct file_info *f = INTTYPE_TO_PTR(fi->fh);
 
 	switch (f->type) {
 	case LXC_TYPE_PROC_MEMINFO:
@@ -1212,7 +1212,7 @@ int proc_read(const char *path, char *buf, size_t size, off_t offset,
 		return proc_swaps_read(buf, size, offset, fi);
 	case LXC_TYPE_PROC_LOADAVG:
 		return proc_loadavg_read(buf, size, offset, fi);
-	default:
-		return -EINVAL;
 	}
+
+	return -EINVAL;
 }
diff --git a/proc_loadavg.c b/proc_loadavg.c
index 3947f31..9fc602b 100644
--- a/proc_loadavg.c
+++ b/proc_loadavg.c
@@ -163,7 +163,7 @@ int proc_loadavg_read(char *buf, size_t size, off_t offset,
 		      struct fuse_file_info *fi)
 {
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	pid_t initpid;
 	char *cg;
 	size_t total_len = 0;
diff --git a/sysfs_fuse.c b/sysfs_fuse.c
index 9ac6639..599790d 100644
--- a/sysfs_fuse.c
+++ b/sysfs_fuse.c
@@ -49,7 +49,7 @@ static int sys_devices_system_cpu_online_read(char *buf, size_t size,
 					      struct fuse_file_info *fi)
 {
 	struct fuse_context *fc = fuse_get_context();
-	struct file_info *d = (struct file_info *)fi->fh;
+	struct file_info *d = INTTYPE_TO_PTR(fi->fh);
 	char *cache = d->buf;
 	char *cg;
 	char *cpuset = NULL;
@@ -234,7 +234,7 @@ int sys_open(const char *path, struct fuse_file_info *fi)
 	/* set actual size to buffer size */
 	info->size = info->buflen;
 
-	fi->fh = (unsigned long)info;
+	fi->fh = PTR_TO_UINT64(info);
 	return 0;
 }
 
@@ -270,7 +270,7 @@ int sys_releasedir(const char *path, struct fuse_file_info *fi)
 int sys_read(const char *path, char *buf, size_t size, off_t offset,
 	     struct fuse_file_info *fi)
 {
-	struct file_info *f = (struct file_info *)fi->fh;
+	struct file_info *f = INTTYPE_TO_PTR(fi->fh);
 
 	switch (f->type) {
 	case LXC_TYPE_SYS_DEVICES_SYSTEM_CPU_ONLINE:
diff --git a/utils.c b/utils.c
index 6422b09..50c940f 100644
--- a/utils.c
+++ b/utils.c
@@ -157,8 +157,9 @@ int preserve_ns(const int pid, const char *ns)
 
 void do_release_file_info(struct fuse_file_info *fi)
 {
-	struct file_info *f = (struct file_info *)fi->fh;
+	struct file_info *f;
 
+	f = INTTYPE_TO_PTR(fi->fh);
 	if (!f)
 		return;
 

From 2b8eff1d5dd755c07208e5aa1fab82bf65ffe78d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 12:26:12 +0100
Subject: [PATCH 3/4] proc_fuse: use correct format specifier

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 bindings.c     |  6 ++---
 proc_cpuview.c | 59 +++++++++++++++++++++++++-------------------------
 proc_fuse.c    | 20 ++++++++---------
 3 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/bindings.c b/bindings.c
index d365ce6..e4a8d15 100644
--- a/bindings.c
+++ b/bindings.c
@@ -692,10 +692,8 @@ static bool cgfs_setup_controllers(void)
 	if (!cgfs_prepare_mounts())
 		return false;
 
-	if (!cgfs_mount_hierarchies()) {
-		lxcfs_error("%s\n", "Failed to set up private lxcfs cgroup mounts.");
-		return false;
-	}
+	if (!cgfs_mount_hierarchies())
+		return log_error_errno(false, errno, "Failed to set up private lxcfs cgroup mounts");
 
 	if (!permute_root())
 		return false;
diff --git a/proc_cpuview.c b/proc_cpuview.c
index 330438a..5d29a5c 100644
--- a/proc_cpuview.c
+++ b/proc_cpuview.c
@@ -372,8 +372,8 @@ static struct cg_proc_stat *find_or_create_proc_stat_node(struct cpuacct_usage *
 	return node;
 }
 
-static void add_cpu_usage(unsigned long *surplus, struct cpuacct_usage *usage,
-			  unsigned long *counter, unsigned long threshold)
+static void add_cpu_usage(uint64_t *surplus, struct cpuacct_usage *usage,
+			  uint64_t *counter, uint64_t threshold)
 {
 	unsigned long free_space, to_add;
 
@@ -441,7 +441,7 @@ static bool read_cpu_cfs_param(const char *cg, const char *param, int64_t *value
 	if (!cgroup_ops->get(cgroup_ops, "cpu", cg, file, &str))
 		return false;
 
-	if (sscanf(str, "%ld", value) != 1)
+	if (sscanf(str, "%"PRId64, value) != 1)
 		return false;
 
 	return true;
@@ -534,11 +534,11 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 	int curcpu = -1; /* cpu numbering starts at 0 */
 	int physcpu, i;
 	int max_cpus = max_cpu_count(cg), cpu_cnt = 0;
-	unsigned long user = 0, nice = 0, system = 0, idle = 0, iowait = 0,
-		      irq = 0, softirq = 0, steal = 0, guest = 0, guest_nice = 0;
-	unsigned long user_sum = 0, system_sum = 0, idle_sum = 0;
-	unsigned long user_surplus = 0, system_surplus = 0;
-	unsigned long total_sum, threshold;
+	uint64_t user = 0, nice = 0, system = 0, idle = 0, iowait = 0, irq = 0,
+		 softirq = 0, steal = 0, guest = 0, guest_nice = 0;
+	uint64_t user_sum = 0, system_sum = 0, idle_sum = 0;
+	uint64_t user_surplus = 0, system_surplus = 0;
+	uint64_t total_sum, threshold;
 	struct cg_proc_stat *stat_node;
 	int nprocs = get_nprocs_conf();
 
@@ -583,7 +583,7 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 
 		cg_cpu_usage[curcpu].online = true;
 
-		ret = sscanf(line, "%*s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu",
+		ret = sscanf(line, "%*s %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 "lu",
 			   &user,
 			   &nice,
 			   &system,
@@ -605,9 +605,8 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 			cg_cpu_usage[curcpu].idle = idle + (all_used - cg_used);
 
 		} else {
-			lxcfs_error("cpu%d from %s has unexpected cpu time: %lu in /proc/stat, "
-					"%lu in cpuacct.usage_all; unable to determine idle time\n",
-					curcpu, cg, all_used, cg_used);
+			lxcfs_error("cpu%d from %s has unexpected cpu time: %" PRIu64 " in /proc/stat, %" PRIu64 " in cpuacct.usage_all; unable to determine idle time",
+				    curcpu, cg, all_used, cg_used);
 			cg_cpu_usage[curcpu].idle = idle;
 		}
 	}
@@ -664,11 +663,11 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 
 	/* Calculate usage counters of visible CPUs */
 	if (max_cpus > 0) {
-		unsigned long diff_user = 0;
-		unsigned long diff_system = 0;
-		unsigned long diff_idle = 0;
-		unsigned long max_diff_idle = 0;
-		unsigned long max_diff_idle_index = 0;
+		uint64_t diff_user = 0;
+		uint64_t diff_system = 0;
+		uint64_t diff_idle = 0;
+		uint64_t max_diff_idle = 0;
+		uint64_t max_diff_idle_index = 0;
 		double exact_cpus;
 
 		/* threshold = maximum usage per cpu, including idle */
@@ -765,10 +764,9 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 
 	/* Render the file */
 	/* cpu-all */
-	l = snprintf(buf, buf_size, "cpu  %lu 0 %lu %lu 0 0 0 0 0 0\n",
-			user_sum,
-			system_sum,
-			idle_sum);
+	l = snprintf(buf, buf_size,
+		     "cpu  %" PRIu64 " 0 %" PRIu64 " %" PRIu64 " 0 0 0 0 0 0\n",
+		     user_sum, system_sum, idle_sum);
 	lxcfs_v("cpu-all: %s\n", buf);
 
 	if (l < 0) {
@@ -794,11 +792,11 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 		if (max_cpus > 0 && i == max_cpus)
 			break;
 
-		l = snprintf(buf, buf_size, "cpu%d %lu 0 %lu %lu 0 0 0 0 0 0\n",
-				i,
-				stat_node->view[curcpu].user,
-				stat_node->view[curcpu].system,
-				stat_node->view[curcpu].idle);
+		l = snprintf(buf, buf_size, "cpu%d %" PRIu64 " 0 %" PRIu64 " %" PRIu64 " 0 0 0 0 0 0\n",
+			     i,
+			     stat_node->view[curcpu].user,
+			     stat_node->view[curcpu].system,
+			     stat_node->view[curcpu].idle);
 		lxcfs_v("cpu: %s\n", buf);
 
 		if (l < 0) {
@@ -1088,8 +1086,8 @@ int read_cpuacct_usage_all(char *cg, char *cpuset,
 
 		must_strcat(&data, &sz, &asz, "cpu user system\n");
 
-		while (sscanf(usage_str + read_pos, "%lu %n", &cg_user, &read_cnt) > 0) {
-			lxcfs_debug("i: %d, cg_user: %lu, read_pos: %d, read_cnt: %d\n", i, cg_user, read_pos, read_cnt);
+		while (sscanf(usage_str + read_pos, "%" PRIu64 " %n", &cg_user, &read_cnt) > 0) {
+			lxcfs_debug("i: %d, cg_user: %" PRIu64 ", read_pos: %d, read_cnt: %d\n", i, cg_user, read_pos, read_cnt);
 			must_strcat(&data, &sz, &asz, "%d %lu 0\n", i, cg_user);
 			i++;
 			read_pos += read_cnt;
@@ -1109,8 +1107,9 @@ int read_cpuacct_usage_all(char *cg, char *cpuset,
 	read_pos += read_cnt;
 
 	for (i = 0, j = 0; i < cpucount; i++) {
-		ret = sscanf(usage_str + read_pos, "%d %lu %lu\n%n", &cg_cpu, &cg_user,
-				&cg_system, &read_cnt);
+		ret = sscanf(usage_str + read_pos,
+			     "%d %" PRIu64 " %" PRIu64 "\n%n", &cg_cpu,
+			     &cg_user, &cg_system, &read_cnt);
 
 		if (ret == EOF)
 			break;
diff --git a/proc_fuse.c b/proc_fuse.c
index 0117fee..037fa32 100644
--- a/proc_fuse.c
+++ b/proc_fuse.c
@@ -762,10 +762,8 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
 	 * If the cpuacct cgroup is present, it is used to calculate the container's
 	 * CPU usage. If not, values from the host's /proc/stat are used.
 	 */
-	if (read_cpuacct_usage_all(cg, cpuset, &cg_cpu_usage, &cg_cpu_usage_size) != 0) {
-		lxcfs_v("%s\n", "proc_stat_read failed to read from cpuacct, "
-				"falling back to the host's /proc/stat");
-	}
+	if (read_cpuacct_usage_all(cg, cpuset, &cg_cpu_usage, &cg_cpu_usage_size) != 0)
+		lxcfs_v("%s\n", "proc_stat_read failed to read from cpuacct, falling back to the host's /proc/stat");
 
 	f = fopen("/proc/stat", "r");
 	if (!f)
@@ -813,7 +811,7 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
 			continue;
 		if (!cpu_in_cpuset(physcpu, cpuset))
 			continue;
-		curcpu ++;
+		curcpu++;
 
 		ret = sscanf(line, "%*s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu",
 			   &user,
@@ -861,15 +859,15 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
 				new_idle = idle + (all_used - cg_used);
 
 			} else {
-				lxcfs_error("cpu%d from %s has unexpected cpu time: %lu in /proc/stat, "
-						"%lu in cpuacct.usage_all; unable to determine idle time\n",
-						curcpu, cg, all_used, cg_used);
+				lxcfs_error("cpu%d from %s has unexpected cpu time: %" PRIu64 " in /proc/stat, %" PRIu64 " in cpuacct.usage_all; unable to determine idle time",
+					    curcpu, cg, all_used, cg_used);
 				new_idle = idle;
 			}
 
-			l = snprintf(cache, cache_size, "cpu%d %lu 0 %lu %lu 0 0 0 0 0 0\n",
-					curcpu, cg_cpu_usage[physcpu].user, cg_cpu_usage[physcpu].system,
-					new_idle);
+			l = snprintf(cache, cache_size,
+				     "cpu%d %" PRIu64 " 0 %" PRIu64 " %" PRIu64 " 0 0 0 0 0 0\n",
+				     curcpu, cg_cpu_usage[physcpu].user,
+				     cg_cpu_usage[physcpu].system, new_idle);
 
 			if (l < 0) {
 				perror("Error writing to cache");

From 1aa8983063cba544ece03a25eec987407743707c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 27 Feb 2020 13:07:05 +0100
Subject: [PATCH 4/4] cgroup_utils: fix mounting

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

diff --git a/cgroups/cgroup_utils.c b/cgroups/cgroup_utils.c
index 9e63578..896ed43 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_NOFOLLOW | O_CLOEXEC);
+	newfd = openat(dirfd, nextpath, O_RDONLY | O_CLOEXEC);
 	if (newfd >= 0) /* Was not a symlink, all good. */
-		return newfd;
+		return move_fd(newfd);
 
 	if (errno == ELOOP)
-		return newfd;
+		return move_fd(newfd);
 
 	if (errno == EPERM || errno == EACCES) {
 		/* We're not root (cause we got EPERM) so try opening with
@@ -310,7 +310,7 @@ static int open_if_safe(int dirfd, const char *nextpath)
 			 */
 			int ret = check_symlink(newfd);
 			if (ret < 0)
-				newfd = ret;
+				return -1;
 		}
 	}
 
@@ -371,7 +371,7 @@ static int open_without_symlink(const char *target, const char *prefix_skip)
 		return -1;
 
 	for (;;) {
-		__do_close_prot_errno int newfd = -EBADF;
+		int newfd;
 		char *nextpath;
 
 		nextpath = get_nextpath(dup, &curlen, fulllen);
@@ -379,11 +379,10 @@ static int open_without_symlink(const char *target, const char *prefix_skip)
 			return move_fd(dirfd);
 
 		newfd = open_if_safe(dirfd, nextpath);
-		if (newfd < 0)
-			return -1;
-
 		close_prot_errno_disarm(dirfd);
 		dirfd = newfd;
+		if (newfd < 0)
+			return -1;
 	}
 
 	return move_fd(dirfd);


More information about the lxc-devel mailing list