[lxc-devel] [lxc/master] macro: add new macro header and caps: bugfixes

brauner on Github lxc-bot at linuxcontainers.org
Sat Aug 4 18:18:59 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180804/27bae9fa/attachment.bin>
-------------- next part --------------
From 279c45eed3b9f6a81a4298d543d272b203614027 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 4 Aug 2018 20:11:58 +0200
Subject: [PATCH 1/2] macro: add new macro header

This allows us to use a bunch of macros in our static build for init.lxc.static
without having to link against all of utils.{c,h}.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/Makefile.am |   3 +
 src/lxc/macro.h     | 139 ++++++++++++++++++++++++++++++++++++++++++++
 src/lxc/utils.h     | 117 +------------------------------------
 3 files changed, 143 insertions(+), 116 deletions(-)
 create mode 100644 src/lxc/macro.h

diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index 1359eb3e4..d5b6e8180 100644
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -16,6 +16,7 @@ noinst_HEADERS = attach.h \
 		 log.h \
 		 lxc.h \
 		 lxclock.h \
+		 macro.h \
 		 monitor.h \
 		 namespace.h \
 		 start.h \
@@ -99,6 +100,7 @@ liblxc_la_SOURCES = af_unix.c af_unix.h \
 		    lxccontainer.c lxccontainer.h \
 		    lxclock.c lxclock.h \
 		    lxcseccomp.h \
+		    macro.h \
 		    mainloop.c mainloop.h \
 		    namespace.c namespace.h \
 		    nl.c nl.h \
@@ -342,6 +344,7 @@ init_lxc_static_SOURCES = cmd/lxc_init.c \
 			  error.c error.h \
 			  initutils.c initutils.h \
 			  log.c log.h \
+			  macro.h \
 			  namespace.c namespace.h \
 			  parse.c parse.h
 
diff --git a/src/lxc/macro.h b/src/lxc/macro.h
new file mode 100644
index 000000000..d2333bf94
--- /dev/null
+++ b/src/lxc/macro.h
@@ -0,0 +1,139 @@
+/* liblxcapi
+ *
+ * Copyright © 2018 Christian Brauner <christian.brauner at ubuntu.com>.
+ * Copyright © 2018 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef __LXC_MACRO_H
+#define __LXC_MACRO_H
+
+/* Define __S_ISTYPE if missing from the C library. */
+#ifndef __S_ISTYPE
+#define __S_ISTYPE(mode, mask) (((mode)&S_IFMT) == (mask))
+#endif
+
+#if HAVE_LIBCAP
+#ifndef CAP_SETFCAP
+#define CAP_SETFCAP 31
+#endif
+
+#ifndef CAP_MAC_OVERRIDE
+#define CAP_MAC_OVERRIDE 32
+#endif
+
+#ifndef CAP_MAC_ADMIN
+#define CAP_MAC_ADMIN 33
+#endif
+#endif
+
+#ifndef PR_CAPBSET_DROP
+#define PR_CAPBSET_DROP 24
+#endif
+
+#ifndef LO_FLAGS_AUTOCLEAR
+#define LO_FLAGS_AUTOCLEAR 4
+#endif
+
+#ifndef CAP_SETUID
+#define CAP_SETUID 7
+#endif
+
+#ifndef CAP_SETGID
+#define CAP_SETGID 6
+#endif
+
+/* needed for cgroup automount checks, regardless of whether we
+ * have included linux/capability.h or not */
+#ifndef CAP_SYS_ADMIN
+#define CAP_SYS_ADMIN 21
+#endif
+
+#ifndef CGROUP_SUPER_MAGIC
+#define CGROUP_SUPER_MAGIC 0x27e0eb
+#endif
+
+#ifndef CGROUP2_SUPER_MAGIC
+#define CGROUP2_SUPER_MAGIC 0x63677270
+#endif
+
+/* Useful macros */
+/* Maximum number for 64 bit integer is a string with 21 digits: 2^64 - 1 = 21 */
+#define LXC_NUMSTRLEN64 21
+#define LXC_LINELEN 4096
+#define LXC_IDMAPLEN 4096
+#define LXC_MAX_BUFFER 4096
+/* /proc/       =    6
+ *                +
+ * <pid-as-str> =   LXC_NUMSTRLEN64
+ *                +
+ * /fd/         =    4
+ *                +
+ * <fd-as-str>  =   LXC_NUMSTRLEN64
+ *                +
+ * \0           =    1
+ */
+#define LXC_PROC_PID_FD_LEN (6 + LXC_NUMSTRLEN64 + 4 + LXC_NUMSTRLEN64 + 1)
+
+/* loop devices */
+#ifndef LO_FLAGS_AUTOCLEAR
+#define LO_FLAGS_AUTOCLEAR 4
+#endif
+
+#ifndef LOOP_CTL_GET_FREE
+#define LOOP_CTL_GET_FREE 0x4C82
+#endif
+
+/* memfd_create() */
+#ifndef MFD_CLOEXEC
+#define MFD_CLOEXEC 0x0001U
+#endif
+
+#ifndef MFD_ALLOW_SEALING
+#define MFD_ALLOW_SEALING 0x0002U
+#endif
+
+/**
+ * BUILD_BUG_ON - break compile if a condition is true.
+ * @condition: the condition which the compiler should know is false.
+ *
+ * If you have some code which relies on certain constants being equal, or
+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * detect if someone changes it.
+ *
+ * The implementation uses gcc's reluctance to create a negative array, but
+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
+ * to inline functions).  So as a fallback we use the optimizer; if it can't
+ * prove the condition is false, it will cause a link error on the undefined
+ * "__build_bug_on_failed".  This error message can be harder to track down
+ * though, hence the two different methods.
+ */
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON(condition)					\
+	do {							\
+		((void)sizeof(char[1 - 2*!!(condition)]));	\
+		if (condition) __build_bug_on_failed = 1;	\
+	} while(0)
+#endif
+
+#define lxc_iterate_parts(__iterator, __splitme, __separators)                  \
+	for (char *__p = NULL, *__it = strtok_r(__splitme, __separators, &__p); \
+	     (__iterator = __it);                                               \
+	     __iterator = __it = strtok_r(NULL, __separators, &__p))
+
+#endif /* __LXC_MACRO_H */
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 7d672b777..fa78721be 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -45,73 +45,7 @@
 #endif
 
 #include "initutils.h"
-
-/* Define __S_ISTYPE if missing from the C library. */
-#ifndef __S_ISTYPE
-#define __S_ISTYPE(mode, mask) (((mode)&S_IFMT) == (mask))
-#endif
-
-#if HAVE_LIBCAP
-#ifndef CAP_SETFCAP
-#define CAP_SETFCAP 31
-#endif
-
-#ifndef CAP_MAC_OVERRIDE
-#define CAP_MAC_OVERRIDE 32
-#endif
-
-#ifndef CAP_MAC_ADMIN
-#define CAP_MAC_ADMIN 33
-#endif
-#endif
-
-#ifndef PR_CAPBSET_DROP
-#define PR_CAPBSET_DROP 24
-#endif
-
-#ifndef LO_FLAGS_AUTOCLEAR
-#define LO_FLAGS_AUTOCLEAR 4
-#endif
-
-#ifndef CAP_SETUID
-#define CAP_SETUID 7
-#endif
-
-#ifndef CAP_SETGID
-#define CAP_SETGID 6
-#endif
-
-/* needed for cgroup automount checks, regardless of whether we
- * have included linux/capability.h or not */
-#ifndef CAP_SYS_ADMIN
-#define CAP_SYS_ADMIN 21
-#endif
-
-#ifndef CGROUP_SUPER_MAGIC
-#define CGROUP_SUPER_MAGIC 0x27e0eb
-#endif
-
-#ifndef CGROUP2_SUPER_MAGIC
-#define CGROUP2_SUPER_MAGIC 0x63677270
-#endif
-
-/* Useful macros */
-/* Maximum number for 64 bit integer is a string with 21 digits: 2^64 - 1 = 21 */
-#define LXC_NUMSTRLEN64 21
-#define LXC_LINELEN 4096
-#define LXC_IDMAPLEN 4096
-#define LXC_MAX_BUFFER 4096
-/* /proc/       =    6
- *                +
- * <pid-as-str> =   LXC_NUMSTRLEN64
- *                +
- * /fd/         =    4
- *                +
- * <fd-as-str>  =   LXC_NUMSTRLEN64
- *                +
- * \0           =    1
- */
-#define LXC_PROC_PID_FD_LEN (6 + LXC_NUMSTRLEN64 + 4 + LXC_NUMSTRLEN64 + 1)
+#include "macro.h"
 
 /* returns 1 on success, 0 if there were any failures */
 extern int lxc_rmdir_onedev(const char *path, const char *exclude);
@@ -265,24 +199,6 @@ static inline int signalfd(int fd, const sigset_t *mask, int flags)
 }
 #endif
 
-/* loop devices */
-#ifndef LO_FLAGS_AUTOCLEAR
-#define LO_FLAGS_AUTOCLEAR 4
-#endif
-
-#ifndef LOOP_CTL_GET_FREE
-#define LOOP_CTL_GET_FREE 0x4C82
-#endif
-
-/* memfd_create() */
-#ifndef MFD_CLOEXEC
-#define MFD_CLOEXEC 0x0001U
-#endif
-
-#ifndef MFD_ALLOW_SEALING
-#define MFD_ALLOW_SEALING 0x0002U
-#endif
-
 #ifndef HAVE_MEMFD_CREATE
 static inline int memfd_create(const char *name, unsigned int flags) {
 	#ifndef __NR_memfd_create
@@ -359,32 +275,6 @@ extern struct lxc_popen_FILE *lxc_popen(const char *command);
  */
 extern int lxc_pclose(struct lxc_popen_FILE *fp);
 
-/**
- * BUILD_BUG_ON - break compile if a condition is true.
- * @condition: the condition which the compiler should know is false.
- *
- * If you have some code which relies on certain constants being equal, or
- * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
- * detect if someone changes it.
- *
- * The implementation uses gcc's reluctance to create a negative array, but
- * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
- * to inline functions).  So as a fallback we use the optimizer; if it can't
- * prove the condition is false, it will cause a link error on the undefined
- * "__build_bug_on_failed".  This error message can be harder to track down
- * though, hence the two different methods.
- */
-#ifndef __OPTIMIZE__
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
-#else
-extern int __build_bug_on_failed;
-#define BUILD_BUG_ON(condition)					\
-	do {							\
-		((void)sizeof(char[1 - 2*!!(condition)]));	\
-		if (condition) __build_bug_on_failed = 1;	\
-	} while(0)
-#endif
-
 /*
  * wait on a child we forked
  */
@@ -620,9 +510,4 @@ extern int lxc_set_death_signal(int signal);
 extern int fd_cloexec(int fd, bool cloexec);
 extern int recursive_destroy(char *dirname);
 
-#define lxc_iterate_parts(__iterator, __splitme, __separators)                  \
-	for (char *__p = NULL, *__it = strtok_r(__splitme, __separators, &__p); \
-	     (__iterator = __it);                                               \
-	     __iterator = __it = strtok_r(NULL, __separators, &__p))
-
 #endif /* __LXC_UTILS_H */

From 3ce2fa23f44756c6cc2a4d4f24c7cd1ae3fe3074 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 4 Aug 2018 20:12:56 +0200
Subject: [PATCH 2/2] caps: bugfixes

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/caps.c | 135 ++++++++++++++++++++++++++-----------------------
 1 file changed, 72 insertions(+), 63 deletions(-)

diff --git a/src/lxc/caps.c b/src/lxc/caps.c
index 344a3389b..83ab07fb2 100644
--- a/src/lxc/caps.c
+++ b/src/lxc/caps.c
@@ -66,51 +66,52 @@ lxc_log_define(caps, lxc);
 int lxc_caps_down(void)
 {
 	cap_t caps;
-	int ret;
+	int ret = -1;
 
-	/* when we are run as root, we don't want to play
-	 * with the capabilities */
+	/* When we are root, we don't want to play with capabilities. */
 	if (!getuid())
 		return 0;
 
 	caps = cap_get_proc();
 	if (!caps) {
-		SYSERROR("Failed to cap_get_proc");
-		return -1;
+		SYSERROR("Failed to retrieve capabilities");
+		return ret;
 	}
 
 	ret = cap_clear_flag(caps, CAP_EFFECTIVE);
 	if (ret) {
-		SYSERROR("Failed to cap_clear_flag");
-		goto out;
+		SYSERROR("Failed to clear effective capabilities");
+		goto on_error;
 	}
 
 	ret = cap_set_proc(caps);
 	if (ret) {
-		SYSERROR("Failed to cap_set_proc");
-		goto out;
+		SYSERROR("Failed to change effective capabilities");
+		goto on_error;
 	}
 
-out:
+	ret = 0;
+
+on_error:
 	cap_free(caps);
-	return 0;
+
+	return ret;
 }
 
 int lxc_caps_up(void)
 {
 	cap_t caps;
 	cap_value_t cap;
-	int ret;
+	int ret = -1;
 
-	/* when we are run as root, we don't want to play
-	 * with the capabilities */
+	/* When we are root, we don't want to play with capabilities. */
 	if (!getuid())
 		return 0;
 
 	caps = cap_get_proc();
 	if (!caps) {
-		SYSERROR("Failed to cap_get_proc");
-		return -1;
+		SYSERROR("Failed to retrieve capabilities");
+		return ret;
 	}
 
 	for (cap = 0; cap <= CAP_LAST_CAP; cap++) {
@@ -119,30 +120,34 @@ int lxc_caps_up(void)
 		ret = cap_get_flag(caps, cap, CAP_PERMITTED, &flag);
 		if (ret) {
 			if (errno == EINVAL) {
-				INFO("Last supported cap was %d", cap-1);
+				INFO("Last supported cap was %d", cap - 1);
 				break;
 			} else {
-				SYSERROR("Failed to cap_get_flag");
-				goto out;
+				SYSERROR("Failed to retrieve setting for "
+					 "permitted capability %d", cap - 1);
+				goto on_error;
 			}
 		}
 
 		ret = cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap, flag);
 		if (ret) {
-			SYSERROR("Failed to cap_set_flag");
-			goto out;
+			SYSERROR("Failed to set effective capability %d", cap - 1);
+			goto on_error;
 		}
 	}
 
 	ret = cap_set_proc(caps);
 	if (ret) {
-		SYSERROR("Failed to cap_set_proc");
-		goto out;
+		SYSERROR("Failed to change effective capabilities");
+		goto on_error;
 	}
 
-out:
+	ret = 0;
+
+on_error:
 	cap_free(caps);
-	return 0;
+
+	return ret;
 }
 
 int lxc_ambient_caps_up(void)
@@ -153,7 +158,7 @@ int lxc_ambient_caps_up(void)
 	int last_cap = CAP_LAST_CAP;
 	char *cap_names = NULL;
 
-	/* When we are run as root, we don't want to play with the capabilities. */
+	/* When we are root, we don't want to play with capabilities. */
 	if (!getuid())
 		return 0;
 
@@ -220,7 +225,7 @@ int lxc_ambient_caps_down(void)
 	cap_t caps;
 	cap_value_t cap;
 
-	/* When we are run as root, we don't want to play with the capabilities. */
+	/* When we are root, we don't want to play with capabilities. */
 	if (!getuid())
 		return 0;
 
@@ -257,79 +262,84 @@ int lxc_ambient_caps_down(void)
 
 int lxc_caps_init(void)
 {
-	uid_t uid = getuid();
-	gid_t gid = getgid();
-	uid_t euid = geteuid();
+	uid_t euid, uid;
 
-	if (!uid) {
-		INFO("command is run as 'root'");
+	uid = getuid();
+	if (!uid)
 		return 0;
-	}
 
+	euid = geteuid();
 	if (uid && !euid) {
-		INFO("command is run as setuid root (uid : %d)", uid);
+		int ret;
+		gid_t gid;
+
+		INFO("Command is run as setuid root (uid: %d)", uid);
 
-		if (prctl(PR_SET_KEEPCAPS, 1)) {
-			SYSERROR("Failed to 'PR_SET_KEEPCAPS'");
+		ret = prctl(PR_SET_KEEPCAPS, 1);
+		if (ret < 0) {
+			SYSERROR("Failed to set PR_SET_KEEPCAPS");
 			return -1;
 		}
 
-		if (setresgid(gid, gid, gid)) {
-			SYSERROR("Failed to change gid to '%d'", gid);
+		gid = getgid();
+		ret = setresgid(gid, gid, gid);
+		if (ret < 0) {
+			SYSERROR("Failed to change rgid, egid, and sgid to %d", gid);
 			return -1;
 		}
 
-		if (setresuid(uid, uid, uid)) {
-			SYSERROR("Failed to change uid to '%d'", uid);
+		ret = setresuid(uid, uid, uid);
+		if (ret < 0) {
+			SYSERROR("Failed to change ruid, euid, and suid to %d", uid);
 			return -1;
 		}
 
-		if (lxc_caps_up()) {
+		ret = lxc_caps_up();
+		if (ret < 0) {
 			SYSERROR("Failed to restore capabilities");
 			return -1;
 		}
 	}
 
 	if (uid == euid)
-		INFO("command is run as user '%d'", uid);
+		INFO("Command is run with uid %d", uid);
 
 	return 0;
 }
 
 static int _real_caps_last_cap(void)
 {
-	int fd;
-	int result = -1;
+	int fd, result = -1;
 
-	/* try to get the maximum capability over the kernel
-	* interface introduced in v3.2 */
-	fd = open("/proc/sys/kernel/cap_last_cap", O_RDONLY);
+	/* Try to get the maximum capability over the kernel interface
+	 * introduced in v3.2.
+	 */
+	fd = open("/proc/sys/kernel/cap_last_cap", O_RDONLY | O_CLOEXEC);
 	if (fd >= 0) {
-		char buf[32];
+		ssize_t n;
 		char *ptr;
-		int n;
+		char buf[LXC_NUMSTRLEN64 + 1];
 
 	again:
-		n = read(fd, buf, 31);
+		n = read(fd, buf, LXC_NUMSTRLEN64);
 		if (n < 0 && errno == EINTR) {
 			goto again;
 		} else if (n >= 0) {
 			buf[n] = '\0';
-			errno = 0;
 
+			errno = 0;
 			result = strtol(buf, &ptr, 10);
 			if (!ptr || (*ptr != '\0' && *ptr != '\n') || errno != 0)
 				result = -1;
 		}
 
 		close(fd);
-	}
-
-	/* try to get it manually by trying to get the status of
-	* each capability indiviually from the kernel */
-	if (result < 0) {
+	} else {
 		int cap = 0;
 
+		/* Try to get it manually by trying to get the status of each
+		 * capability individually from the kernel.
+		 */
 		while (prctl(PR_CAPBSET_READ, cap) >= 0)
 			cap++;
 
@@ -356,7 +366,7 @@ static bool lxc_cap_is_set(cap_t caps, cap_value_t cap, cap_flag_t flag)
 
 	ret = cap_get_flag(caps, cap, flag, &flagval);
 	if (ret < 0) {
-		SYSERROR("Failed to perform cap_get_flag()");
+		SYSERROR("Failed to retrieve current setting for capability %d", cap);
 		return false;
 	}
 
@@ -365,7 +375,7 @@ static bool lxc_cap_is_set(cap_t caps, cap_value_t cap, cap_flag_t flag)
 
 bool lxc_file_cap_is_set(const char *path, cap_value_t cap, cap_flag_t flag)
 {
-	#if LIBCAP_SUPPORTS_FILE_CAPABILITIES
+#if LIBCAP_SUPPORTS_FILE_CAPABILITIES
 	bool cap_is_set;
 	cap_t caps;
 
@@ -377,7 +387,7 @@ bool lxc_file_cap_is_set(const char *path, cap_value_t cap, cap_flag_t flag)
 		 * case errno will be set to ENODATA.
 		 */
 		if (errno != ENODATA)
-			SYSERROR("Failed to perform cap_get_file()");
+			SYSERROR("Failed to retrieve capabilities for file %s", path);
 
 		return false;
 	}
@@ -385,10 +395,10 @@ bool lxc_file_cap_is_set(const char *path, cap_value_t cap, cap_flag_t flag)
 	cap_is_set = lxc_cap_is_set(caps, cap, flag);
 	cap_free(caps);
 	return cap_is_set;
-	#else
+#else
 	errno = ENODATA;
 	return false;
-	#endif
+#endif
 }
 
 bool lxc_proc_cap_is_set(cap_value_t cap, cap_flag_t flag)
@@ -398,7 +408,7 @@ bool lxc_proc_cap_is_set(cap_value_t cap, cap_flag_t flag)
 
 	caps = cap_get_proc();
 	if (!caps) {
-		SYSERROR("Failed to perform cap_get_proc()");
+		SYSERROR("Failed to retrieve capabilities");
 		return false;
 	}
 
@@ -406,5 +416,4 @@ bool lxc_proc_cap_is_set(cap_value_t cap, cap_flag_t flag)
 	cap_free(caps);
 	return cap_is_set;
 }
-
 #endif


More information about the lxc-devel mailing list