[lxc-devel] [lxc/master] confile: make reading config file thread-safe

brauner on Github lxc-bot at linuxcontainers.org
Thu Feb 22 12:07:25 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 569 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180222/8e392f6d/attachment.bin>
-------------- next part --------------
From 607d3153c43b88796f489ee21b4109dc83f551d5 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 22 Feb 2018 13:03:55 +0100
Subject: [PATCH] confile: make reading config file thread-safe

When multiple threads read the same config file (e.g. the global default
config) the file pointer seems to be modified by the various threads. There's
an obvious solution to this problem: m{un}map().

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/Makefile.am        |  2 +-
 src/lxc/cmd/lxc_user_nic.c |  1 +
 src/lxc/confile.c          |  2 +-
 src/lxc/lxccontainer.c     |  1 +
 src/lxc/parse.c            | 79 ++++++++++++++++++++++++++++++++++++++++++++--
 src/lxc/parse.h            | 13 ++++++++
 src/lxc/utils.c            | 27 ----------------
 src/lxc/utils.h            |  7 ----
 8 files changed, 93 insertions(+), 39 deletions(-)

diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index 8e7c64c46..bee953808 100644
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -278,7 +278,7 @@ lxc_checkpoint_SOURCES = tools/lxc_checkpoint.c tools/arguments.c tools/tool_uti
 # Binaries shipping with liblxc
 init_lxc_SOURCES = cmd/lxc_init.c
 lxc_monitord_SOURCES = cmd/lxc_monitord.c
-lxc_user_nic_SOURCES = cmd/lxc_user_nic.c namespace.c network.c
+lxc_user_nic_SOURCES = cmd/lxc_user_nic.c namespace.c network.c parse.c
 lxc_usernsexec_SOURCES = cmd/lxc_usernsexec.c
 
 if ENABLE_DEPRECATED
diff --git a/src/lxc/cmd/lxc_user_nic.c b/src/lxc/cmd/lxc_user_nic.c
index 2a5c3a43a..d071ce0b5 100644
--- a/src/lxc/cmd/lxc_user_nic.c
+++ b/src/lxc/cmd/lxc_user_nic.c
@@ -48,6 +48,7 @@
 #include "config.h"
 #include "namespace.h"
 #include "network.h"
+#include "parse.h"
 #include "utils.h"
 
 #define usernic_debug_stream(stream, format, ...)                              \
diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index a9db53630..39c8ae49d 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -2334,7 +2334,7 @@ int lxc_config_read(const char *file, struct lxc_conf *conf, bool from_include)
 	if (!conf->rcfile)
 		conf->rcfile = strdup(file);
 
-	return lxc_file_for_each_line(file, parse_line, &c);
+	return lxc_file_for_each_line_thread_safe(file, parse_line, &c);
 }
 
 int lxc_config_define_add(struct lxc_list *defines, char *arg)
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 26fdc23bf..e6ad6d501 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -57,6 +57,7 @@
 #include "monitor.h"
 #include "namespace.h"
 #include "network.h"
+#include "parse.h"
 #include "start.h"
 #include "state.h"
 #include "storage.h"
diff --git a/src/lxc/parse.c b/src/lxc/parse.c
index 9242763e9..fe3c7a165 100644
--- a/src/lxc/parse.c
+++ b/src/lxc/parse.c
@@ -20,13 +20,15 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
+
 #define _GNU_SOURCE
 #include <stdio.h>
 #undef _GNU_SOURCE
-#include <string.h>
-#include <stdlib.h>
-#include <errno.h>
 #include <dirent.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
 
 #include "parse.h"
 #include "config.h"
@@ -35,6 +37,77 @@
 
 lxc_log_define(lxc_parse, lxc);
 
+void *lxc_strmmap(void *addr, size_t length, int prot, int flags, int fd,
+		  off_t offset)
+{
+	void *tmp = NULL, *overlap = NULL;
+
+	/* We establish an anonymous mapping that is one byte larger than the
+	 * underlying file. The pages handed to us are zero filled. */
+	tmp = mmap(addr, length + 1, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (tmp == MAP_FAILED)
+		return tmp;
+
+	/* Now we establish a fixed-address mapping starting at the address we
+	 * received from our anonymous mapping and replace all bytes excluding
+	 * the additional \0-byte with the file. This allows us to use normal
+	 * string-handling functions. */
+	overlap = mmap(tmp, length, prot, MAP_FIXED | flags, fd, offset);
+	if (overlap == MAP_FAILED)
+		munmap(tmp, length + 1);
+
+	return overlap;
+}
+
+int lxc_strmunmap(void *addr, size_t length)
+{
+	return munmap(addr, length + 1);
+}
+
+int lxc_file_for_each_line_thread_safe(const char *file, lxc_file_cb callback,
+				       void *data)
+{
+	int fd, ret;
+	char *buf, *line;
+	struct stat st;
+	char *saveptr = NULL;
+
+	fd = open(file, O_RDONLY | O_CLOEXEC);
+	if (fd < 0)
+		return -1;
+
+	ret = fstat(fd, &st);
+	if (ret < 0) {
+		close(fd);
+		return -1;
+	}
+
+	if (st.st_size == 0)
+		return 0;
+
+	buf = lxc_strmmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (buf == MAP_FAILED) {
+		close(fd);
+		return -1;
+	}
+
+	for (; (line = strtok_r(buf, "\n\0", &saveptr)); buf = NULL) {
+		ret = callback(line, data);
+		if (ret) {
+			/* Callback rv > 0 means stop here callback rv < 0 means
+			 * error.
+			 */
+			if (ret < 0)
+				ERROR("Failed to parse config: %s", line);
+			break;
+		}
+	}
+
+	lxc_strmunmap(buf, st.st_size);
+	close(fd);
+	return 0;
+}
+
 int lxc_file_for_each_line(const char *file, lxc_file_cb callback, void *data)
 {
 	FILE *f;
diff --git a/src/lxc/parse.h b/src/lxc/parse.h
index 8f753198b..ad6c49c7a 100644
--- a/src/lxc/parse.h
+++ b/src/lxc/parse.h
@@ -23,6 +23,9 @@
 #ifndef __LXC_PARSE_H
 #define __LXC_PARSE_H
 
+#include <stdio.h>
+#include <sys/types.h>
+
 typedef int (*lxc_dir_cb)(const char *name, const char *directory,
 			  const char *file, void *data);
 
@@ -31,10 +34,20 @@ typedef int (*lxc_file_cb)(char *buffer, void *data);
 extern int lxc_file_for_each_line(const char *file, lxc_file_cb callback,
 				  void* data);
 
+extern int lxc_file_for_each_line_thread_safe(const char *file,
+					      lxc_file_cb callback, void *data);
+
 extern int lxc_char_left_gc(const char *buffer, size_t len);
 
 extern int lxc_char_right_gc(const char *buffer, size_t len);
 
 extern int lxc_is_line_empty(const char *line);
 
+/* mmap() wrapper. lxc_strmmap() will take care to \0-terminate files so that
+ * normal string-handling functions can be used on the buffer. */
+extern void *lxc_strmmap(void *addr, size_t length, int prot, int flags, int fd,
+			 off_t offset);
+/* munmap() wrapper. Use it to free memory mmap()ed with lxc_strmmap(). */
+extern int lxc_strmunmap(void *addr, size_t length);
+
 #endif
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index d7527b429..c824e9b99 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1810,33 +1810,6 @@ int lxc_count_file_lines(const char *fn)
 	return n;
 }
 
-void *lxc_strmmap(void *addr, size_t length, int prot, int flags, int fd,
-		  off_t offset)
-{
-	void *tmp = NULL, *overlap = NULL;
-
-	/* We establish an anonymous mapping that is one byte larger than the
-	 * underlying file. The pages handed to us are zero filled. */
-	tmp = mmap(addr, length + 1, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-	if (tmp == MAP_FAILED)
-		return tmp;
-
-	/* Now we establish a fixed-address mapping starting at the address we
-	 * received from our anonymous mapping and replace all bytes excluding
-	 * the additional \0-byte with the file. This allows us to use normal
-	 * string-handling functions. */
-	overlap = mmap(tmp, length, prot, MAP_FIXED | flags, fd, offset);
-	if (overlap == MAP_FAILED)
-		munmap(tmp, length + 1);
-
-	return overlap;
-}
-
-int lxc_strmunmap(void *addr, size_t length)
-{
-	return munmap(addr, length + 1);
-}
-
 /* Check whether a signal is blocked by a process. */
 /* /proc/pid-to-str/status\0 = (5 + 21 + 7 + 1) */
 #define __PROC_STATUS_LEN (5 + (LXC_NUMSTRLEN64) + 7 + 1)
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 87b5a22b4..f013ef23e 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -426,13 +426,6 @@ extern size_t lxc_array_len(void **array);
 
 extern void **lxc_append_null_to_array(void **array, size_t count);
 
-/* mmap() wrapper. lxc_strmmap() will take care to \0-terminate files so that
- * normal string-handling functions can be used on the buffer. */
-extern void *lxc_strmmap(void *addr, size_t length, int prot, int flags, int fd,
-			 off_t offset);
-/* munmap() wrapper. Use it to free memory mmap()ed with lxc_strmmap(). */
-extern int lxc_strmunmap(void *addr, size_t length);
-
 /* initialize rand with urandom */
 extern int randseed(bool);
 


More information about the lxc-devel mailing list