Merge pull request #2884 from brauner/2019-03-01/kill_fgets

src/lxc/conf.c

@@ -307,7 +307,7 @@ static struct limit_opt limit_opt[] = {
 static int run_buffer(char *buffer)
 {
 	__do_free char *output = NULL;
-	int ret;
+	int fd, ret;
 	struct lxc_popen_FILE *f;
 
 	f = lxc_popen(buffer);
@@ -323,8 +323,25 @@ static int run_buffer(char *buffer)
 		return -1;
 	}
 
-	while (fgets(output, LXC_LOG_BUFFER_SIZE, f->f))
-		DEBUG("Script %s with output: %s", buffer, output);
+	fd = fileno(f->f);
+	if (fd < 0) {
+		SYSERROR("Failed to retrieve underlying file descriptor");
+		lxc_pclose(f);
+		return -1;
+	}
+
+	for (int i = 0; i < 10; i++) {
+		ssize_t bytes_read;
+
+		bytes_read = lxc_read_nointr(fd, output, LXC_LOG_BUFFER_SIZE - 1);
+		if (bytes_read > 0) {
+			output[bytes_read] = '\0';
+			DEBUG("Script %s produced output: %s", buffer, output);
+			continue;
+		}
+
+		break;
+	}
 
 	ret = lxc_pclose(f);
 	if (ret == -1) {
@@ -1347,8 +1364,6 @@ int lxc_chroot(const struct lxc_rootfs *rootfs)
 {
 	__do_free char *nroot = NULL;
 	int i, ret;
-	char *p, *p2;
-	char buf[LXC_LINELEN];
 	char *root = rootfs->mount;
 
 	nroot = realpath(root, NULL);
@@ -1388,7 +1403,10 @@ int lxc_chroot(const struct lxc_rootfs *rootfs)
 	 */
 	for (;;) {
 		__do_fclose FILE *f = NULL;
+		__do_free char *line = NULL;
+		char *slider1, *slider2;
 		int progress = 0;
+		size_t len = 0;
 
 		f = fopen("./proc/self/mountinfo", "r");
 		if (!f) {
@@ -1396,27 +1414,27 @@ int lxc_chroot(const struct lxc_rootfs *rootfs)
 			return -1;
 		}
 
-		while (fgets(buf, LXC_LINELEN, f)) {
-			for (p = buf, i=0; p && i < 4; i++)
-				p = strchr(p+1, ' ');
+		while (getline(&line, &len, f) > 0) {
+			for (slider1 = line, i = 0; slider1 && i < 4; i++)
+				slider1 = strchr(slider1 + 1, ' ');
 
-			if (!p)
+			if (!slider1)
 				continue;
 
-			p2 = strchr(p+1, ' ');
-			if (!p2)
+			slider2 = strchr(slider1 + 1, ' ');
+			if (!slider2)
 				continue;
 
-			*p2 = '\0';
-			*p = '.';
+			*slider2 = '\0';
+			*slider1 = '.';
 
-			if (strcmp(p + 1, "/") == 0)
+			if (strcmp(slider1 + 1, "/") == 0)
 				continue;
 
-			if (strcmp(p + 1, "/proc") == 0)
+			if (strcmp(slider1 + 1, "/proc") == 0)
 				continue;
 
-			ret = umount2(p, MNT_DETACH);
+			ret = umount2(slider1, MNT_DETACH);
 			if (ret == 0)
 				progress++;
 		}

src/lxc/initutils.c

@@ -32,6 +32,7 @@
 #include "initutils.h"
 #include "log.h"
 #include "macro.h"
+#include "memory_utils.h"
 
 #ifndef HAVE_STRLCPY
 #include "include/strlcpy.h"
@@ -115,7 +116,6 @@ const char *lxc_global_config_value(const char *option_name)
 
 	const char * const (*ptr)[2];
 	size_t i;
-	char buf[1024], *p, *p2;
 	FILE *fin = NULL;
 
 	for (i = 0, ptr = options; (*ptr)[0]; ptr++, i++) {
@@ -142,51 +142,64 @@ const char *lxc_global_config_value(const char *option_name)
 	fin = fopen_cloexec(user_config_path, "r");
 	free(user_config_path);
 	if (fin) {
-		while (fgets(buf, 1024, fin)) {
-			if (buf[0] == '#')
+		__do_free char *line = NULL;
+		size_t len = 0;
+		char *slider1, *slider2;
+
+		while (getline(&line, &len, fin) > 0) {
+			if (*line == '#')
 				continue;
-			p = strstr(buf, option_name);
-			if (!p)
+
+			slider1 = strstr(line, option_name);
+			if (!slider1)
 				continue;
+
 			/* see if there was just white space in front
 			 * of the option name
 			 */
-			for (p2 = buf; p2 < p; p2++) {
-				if (*p2 != ' ' && *p2 != '\t')
+			for (slider2 = line; slider2 < slider1; slider2++)
+				if (*slider2 != ' ' && *slider2 != '\t')
 					break;
-			}
-			if (p2 < p)
+
+			if (slider2 < slider1)
 				continue;
-			p = strchr(p, '=');
-			if (!p)
+
+			slider1 = strchr(slider1, '=');
+			if (!slider1)
 				continue;
+
 			/* see if there was just white space after
 			 * the option name
 			 */
-			for (p2 += strlen(option_name); p2 < p; p2++) {
-				if (*p2 != ' ' && *p2 != '\t')
+			for (slider2 += strlen(option_name); slider2 < slider1;
+			     slider2++)
+				if (*slider2 != ' ' && *slider2 != '\t')
 					break;
-			}
-			if (p2 < p)
+
+			if (slider2 < slider1)
 				continue;
-			p++;
-			while (*p && (*p == ' ' || *p == '\t')) p++;
-			if (!*p)
+
+			slider1++;
+			while (*slider1 && (*slider1 == ' ' || *slider1 == '\t'))
+				slider1++;
+
+			if (!*slider1)
 				continue;
 
 			if (strcmp(option_name, "lxc.lxcpath") == 0) {
 				free(user_lxc_path);
-				user_lxc_path = copy_global_config_value(p);
+				user_lxc_path = copy_global_config_value(slider1);
 				remove_trailing_slashes(user_lxc_path);
 				values[i] = user_lxc_path;
 				user_lxc_path = NULL;
 				goto out;
 			}
 
-			values[i] = copy_global_config_value(p);
+			values[i] = copy_global_config_value(slider1);
 			goto out;
 		}
 	}
+
 	/* could not find value, use default */
 	if (strcmp(option_name, "lxc.lxcpath") == 0) {
 		remove_trailing_slashes(user_lxc_path);
@@ -227,62 +240,64 @@ out:
  */
 int setproctitle(char *title)
 {
+	__do_fclose FILE *f = NULL;
+	int i, fd, len;
+	char *buf_ptr;
+	char buf[LXC_LINELEN];
+	int ret = 0;
+	ssize_t bytes_read = 0;
 	static char *proctitle = NULL;
-	char buf[2048], *tmp;
-	FILE *f;
-	int i, len, ret = 0;
 
-	/* We don't really need to know all of this stuff, but unfortunately
+	/*
+	 * We don't really need to know all of this stuff, but unfortunately
 	 * PR_SET_MM_MAP requires us to set it all at once, so we have to
 	 * figure it out anyway.
 	 */
 	unsigned long start_data, end_data, start_brk, start_code, end_code,
-			start_stack, arg_start, arg_end, env_start, env_end,
-			brk_val;
+	    start_stack, arg_start, arg_end, env_start, env_end, brk_val;
 	struct prctl_mm_map prctl_map;
 
 	f = fopen_cloexec("/proc/self/stat", "r");
-	if (!f) {
+	if (!f)
 		return -1;
-	}
 
-	tmp = fgets(buf, sizeof(buf), f);
-	fclose(f);
-	if (!tmp) {
+	fd = fileno(f);
+	if (fd < 0)
 		return -1;
-	}
+
+	bytes_read = lxc_read_nointr(fd, buf, sizeof(buf) - 1);
+	if (bytes_read <= 0)
+		return -1;
+
+	buf[bytes_read] = '\0';
 
 	/* Skip the first 25 fields, column 26-28 are start_code, end_code,
 	 * and start_stack */
-	tmp = strchr(buf, ' ');
+	buf_ptr = strchr(buf, ' ');
 	for (i = 0; i < 24; i++) {
-		if (!tmp)
+		if (!buf_ptr)
 			return -1;
-		tmp = strchr(tmp+1, ' ');
+		buf_ptr = strchr(buf_ptr + 1, ' ');
 	}
-	if (!tmp)
+	if (!buf_ptr)
 		return -1;
 
-	i = sscanf(tmp, "%lu %lu %lu", &start_code, &end_code, &start_stack);
+	i = sscanf(buf_ptr, "%lu %lu %lu", &start_code, &end_code, &start_stack);
 	if (i != 3)
 		return -1;
 
 	/* Skip the next 19 fields, column 45-51 are start_data to arg_end */
 	for (i = 0; i < 19; i++) {
-		if (!tmp)
+		if (!buf_ptr)
 			return -1;
-		tmp = strchr(tmp+1, ' ');
+		buf_ptr = strchr(buf_ptr + 1, ' ');
 	}
 
-	if (!tmp)
+	if (!buf_ptr)
 		return -1;
 
-	i = sscanf(tmp, "%lu %lu %lu %*u %*u %lu %lu",
-		&start_data,
-		&end_data,
-		&start_brk,
-		&env_start,
-		&env_end);
+	i = sscanf(buf_ptr, "%lu %lu %lu %*u %*u %lu %lu", &start_data,
+		   &end_data, &start_brk, &env_start, &env_end);
 	if (i != 5)
 		return -1;
 
@@ -294,32 +309,32 @@ int setproctitle(char *title)
 	if (!proctitle)
 		return -1;
 
-	arg_start = (unsigned long) proctitle;
+	arg_start = (unsigned long)proctitle;
 	arg_end = arg_start + len;
 
 	brk_val = syscall(__NR_brk, 0);
 
-	prctl_map = (struct prctl_mm_map) {
-		.start_code = start_code,
-		.end_code = end_code,
-		.start_stack = start_stack,
-		.start_data = start_data,
-		.end_data = end_data,
-		.start_brk = start_brk,
-		.brk = brk_val,
-		.arg_start = arg_start,
-		.arg_end = arg_end,
-		.env_start = env_start,
-		.env_end = env_end,
-		.auxv = NULL,
-		.auxv_size = 0,
-		.exe_fd = -1,
+	prctl_map = (struct prctl_mm_map){
+	    .start_code = start_code,
+	    .end_code = end_code,
+	    .start_stack = start_stack,
+	    .start_data = start_data,
+	    .end_data = end_data,
+	    .start_brk = start_brk,
+	    .brk = brk_val,
+	    .arg_start = arg_start,
+	    .arg_end = arg_end,
+	    .env_start = env_start,
+	    .env_end = env_end,
+	    .auxv = NULL,
+	    .auxv_size = 0,
+	    .exe_fd = -1,
 	};
 
 	ret = prctl(PR_SET_MM, prctl_arg(PR_SET_MM_MAP), prctl_arg(&prctl_map),
 		    prctl_arg(sizeof(prctl_map)), prctl_arg(0));
 	if (ret == 0)
-		(void)strlcpy((char*)arg_start, title, len);
+		(void)strlcpy((char *)arg_start, title, len);
 	else
 		SYSWARN("Failed to set cmdline");
 

src/lxc/utils.c

@@ -49,6 +49,7 @@
 #include "config.h"
 #include "log.h"
 #include "lxclock.h"
+#include "memory_utils.h"
 #include "namespace.h"
 #include "parse.h"
 #include "raw_syscalls.h"
@@ -641,37 +642,37 @@ uint64_t fnv_64a_buf(void *buf, size_t len, uint64_t hval)
 
 bool is_shared_mountpoint(const char *path)
 {
-	char buf[LXC_LINELEN];
-	FILE *f;
+	__do_fclose FILE *f = NULL;
+	__do_free char *line = NULL;
 	int i;
-	char *p, *p2;
+	size_t len = 0;
 
 	f = fopen("/proc/self/mountinfo", "r");
 	if (!f)
 		return 0;
 
-	while (fgets(buf, LXC_LINELEN, f)) {
-		for (p = buf, i = 0; p && i < 4; i++)
-			p = strchr(p + 1, ' ');
-		if (!p)
+	while (getline(&line, &len, f) > 0) {
+		char *slider1, *slider2;
+
+		for (slider1 = line, i = 0; slider1 && i < 4; i++)
+			slider1 = strchr(slider1 + 1, ' ');
+
+		if (!slider1)
 			continue;
 
-		p2 = strchr(p + 1, ' ');
-		if (!p2)
+		slider2 = strchr(slider1 + 1, ' ');
+		if (!slider2)
 			continue;
 
-		*p2 = '\0';
-		if (strcmp(p + 1, path) == 0) {
+		*slider2 = '\0';
+		if (strcmp(slider1 + 1, path) == 0) {
 			/* This is the path. Is it shared? */
-			p = strchr(p2 + 1, ' ');
-			if (p && strstr(p, "shared:")) {
-				fclose(f);
+			slider1 = strchr(slider2 + 1, ' ');
+			if (slider1 && strstr(slider1, "shared:"))
 				return true;
-			}
 		}
 	}
 
-	fclose(f);
 	return false;
 }
 

src/lxc/utils.h

@@ -61,10 +61,10 @@ static inline int lxc_set_cloexec(int fd)
 	return fcntl(fd, F_SETFD, FD_CLOEXEC);
 }
 
-/* Struct to carry child pid from lxc_popen() to lxc_pclose().
- * Not an opaque struct to allow direct access to the underlying FILE *
- * (i.e., struct lxc_popen_FILE *file; fgets(buf, sizeof(buf), file->f))
- * without additional wrappers.
+/*
+ * Struct to carry child pid from lxc_popen() to lxc_pclose(). Not an opaque
+ * struct to allow direct access to the underlying FILE without additional
+ * wrappers.
  */
 struct lxc_popen_FILE {
 	int pipe;