[pve-devel] [PATCH lxc] seccomp and reboot fixes

Wolfgang Bumiller w.bumiller at proxmox.com
Fri Jul 24 09:11:42 CEST 2015


---
 ...reboot-flag-and-delete-old-veth-on-reboot.patch |  78 +++++++++++
 ...001-seccomp-simplify-and-fix-rule-parsing.patch | 149 +++++++++++++++++++++
 debian/patches/series                              |   2 +
 3 files changed, 229 insertions(+)
 create mode 100644 debian/patches/0001-pass-on-reboot-flag-and-delete-old-veth-on-reboot.patch
 create mode 100644 debian/patches/0001-seccomp-simplify-and-fix-rule-parsing.patch

diff --git a/debian/patches/0001-pass-on-reboot-flag-and-delete-old-veth-on-reboot.patch b/debian/patches/0001-pass-on-reboot-flag-and-delete-old-veth-on-reboot.patch
new file mode 100644
index 0000000..a6b5432
--- /dev/null
+++ b/debian/patches/0001-pass-on-reboot-flag-and-delete-old-veth-on-reboot.patch
@@ -0,0 +1,78 @@
+From 33564919e4af8557c1556a3daeb619922ad8b609 Mon Sep 17 00:00:00 2001
+From: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Date: Fri, 24 Jul 2015 08:21:39 +0200
+Subject: [PATCH] pass on reboot flag and delete old veth on reboot
+
+When using a fixed interface name the recreation of it after
+a reboot caused an EEXIST.
+-) The reboot flag is now kept till after lxc_spawn instead
+of just before lxc_start in order to know whether to delete
+the old interface.
+-) If the reboot flag is set within instantiate_veth and a
+fixed name is used, the interface is now deleted before
+being recreated.
+
+Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
+---
+ src/lxc/conf.c         | 6 ++++--
+ src/lxc/lxccontainer.c | 3 +--
+ src/lxc/start.c        | 2 ++
+ 3 files changed, 7 insertions(+), 4 deletions(-)
+
+diff --git a/src/lxc/conf.c b/src/lxc/conf.c
+index 9870455..ed2ad66 100644
+--- a/src/lxc/conf.c
++++ b/src/lxc/conf.c
+@@ -2613,9 +2613,11 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
+ 	char veth2buf[IFNAMSIZ], *veth2;
+ 	int err;
+ 
+-	if (netdev->priv.veth_attr.pair)
++	if (netdev->priv.veth_attr.pair) {
+ 		veth1 = netdev->priv.veth_attr.pair;
+-	else {
++		if (handler->conf->reboot)
++			lxc_netdev_delete_by_name(veth1);
++	} else {
+ 		err = snprintf(veth1buf, sizeof(veth1buf), "vethXXXXXX");
+ 		if (err >= sizeof(veth1buf)) { /* can't *really* happen, but... */
+ 			ERROR("veth1 name too long");
+diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
+index 1c103e8..b483dd8 100644
+--- a/src/lxc/lxccontainer.c
++++ b/src/lxc/lxccontainer.c
+@@ -760,9 +760,9 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
+ 		pid_fp = NULL;
+ 	}
+ 
+-reboot:
+ 	conf->reboot = 0;
+ 
++reboot:
+ 	if (lxc_check_inherited(conf, daemonize, -1)) {
+ 		ERROR("Inherited fds found");
+ 		ret = 1;
+@@ -774,7 +774,6 @@ reboot:
+ 
+ 	if (conf->reboot) {
+ 		INFO("container requested reboot");
+-		conf->reboot = 0;
+ 		goto reboot;
+ 	}
+ 
+diff --git a/src/lxc/start.c b/src/lxc/start.c
+index 6eded61..2fc026e 100644
+--- a/src/lxc/start.c
++++ b/src/lxc/start.c
+@@ -1173,6 +1173,8 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
+ 		goto out_detach_blockdev;
+ 	}
+ 
++	handler->conf->reboot = 0;
++
+ 	netnsfd = get_netns_fd(handler->pid);
+ 
+ 	err = lxc_poll(name, handler);
+-- 
+2.1.4
+
diff --git a/debian/patches/0001-seccomp-simplify-and-fix-rule-parsing.patch b/debian/patches/0001-seccomp-simplify-and-fix-rule-parsing.patch
new file mode 100644
index 0000000..9b90a5e
--- /dev/null
+++ b/debian/patches/0001-seccomp-simplify-and-fix-rule-parsing.patch
@@ -0,0 +1,149 @@
+From d6417887b93477133a2c600ce755ba3afc843d44 Mon Sep 17 00:00:00 2001
+From: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Date: Thu, 23 Jul 2015 11:10:18 +0200
+Subject: [PATCH] seccomp: simplify and fix rule parsing
+
+1) Two checks on amd64 for whether compat_ctx has already
+been generated were redundant, as compat_ctx is generally
+generated before entering the parsing loop.
+
+2) With introduction of reject_force_umount the check for
+whether the syscall has the same id on both native and
+compat archs results in false behavior as this is an
+internal keyword and thus produces a -1 on
+seccomp_syscall_resolve_name_arch().
+The result was that it was added to the native architecture
+twice and never to the 32 bit architecture, causing it to
+have no effect on 32 bit containers on 64 bit hosts.
+
+3) I do not see a reason to care about whether the syscalls
+have the same number on the two architectures. On the one
+hand this check was there to avoid adding it to two archs
+(and effectively leaving one arch unprotected), while on
+the other hand it seemed to be okay to add it to the
+same arch *twice*.
+
+The entire architecture checking branches are now reduced to
+three simple cases: 'native', 'non-native' and 'all'. With
+'all' adding to both architectures regardless of the syscall
+ID.
+
+Also note that libseccomp had a bug in its architecture
+checking, so architecture related filters weren't working as
+expected before version 2.2.2, which may have contributed to
+the confusion in the original architecture-related code.
+
+Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
+---
+ src/lxc/seccomp.c | 63 ++++++++++++++-----------------------------------------
+ 1 file changed, 16 insertions(+), 47 deletions(-)
+
+diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
+index 108faa0..07dfbc6 100644
+--- a/src/lxc/seccomp.c
++++ b/src/lxc/seccomp.c
+@@ -259,6 +259,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
+ 	uint32_t default_policy_action = -1, default_rule_action = -1, action;
+ 	enum lxc_hostarch_t native_arch = get_hostarch(),
+ 			    cur_rule_arch = native_arch;
++	uint32_t compat_arch = SCMP_ARCH_NATIVE;
+ 
+ 	if (strncmp(line, "blacklist", 9) == 0)
+ 		blacklist = true;
+@@ -288,6 +289,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
+ 
+ 	if (native_arch == lxc_seccomp_arch_amd64) {
+ 		cur_rule_arch = lxc_seccomp_arch_all;
++		compat_arch = SCMP_ARCH_X86;
+ 		compat_ctx = get_new_ctx(lxc_seccomp_arch_i386,
+ 				default_policy_action);
+ 		if (!compat_ctx)
+@@ -324,14 +326,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
+ 					continue;
+ 				}
+ 				cur_rule_arch = lxc_seccomp_arch_i386;
+-				if (native_arch == lxc_seccomp_arch_amd64) {
+-					if (compat_ctx)
+-						continue;
+-					compat_ctx = get_new_ctx(lxc_seccomp_arch_i386,
+-							default_policy_action);
+-					if (!compat_ctx)
+-						goto bad;
+-				}
+ 			} else if (strcmp(line, "[X86_64]") == 0 ||
+ 					strcmp(line, "[x86_64]") == 0) {
+ 				if (native_arch != lxc_seccomp_arch_amd64) {
+@@ -342,14 +336,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
+ 			} else if (strcmp(line, "[all]") == 0 ||
+ 					strcmp(line, "[ALL]") == 0) {
+ 				cur_rule_arch = lxc_seccomp_arch_all;
+-				if (native_arch == lxc_seccomp_arch_amd64 && !compat_ctx) {
+-					if (compat_ctx)
+-						continue;
+-					compat_ctx = get_new_ctx(lxc_seccomp_arch_i386,
+-							default_policy_action);
+-					if (!compat_ctx)
+-						goto bad;
+-				}
+ 			}
+ #ifdef SCMP_ARCH_ARM
+ 			else if (strcmp(line, "[arm]") == 0 ||
+@@ -408,41 +394,24 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
+ 			goto bad_rule;
+ 		}
+ 
+-		/*
+-		 * TODO generalize - if !is_compat_only(native_arch, cur_rule_arch)
+-		 *
+-		 * in other words, the rule is 32-bit only, on 64-bit host;  don't run
+-		 * the rule against the native arch.
+-		 */
+-		if (!(cur_rule_arch == lxc_seccomp_arch_i386 &&
+-			native_arch == lxc_seccomp_arch_amd64)) {
+-			INFO("Adding non-compat rule for %s action %d", line, action);
++		if (cur_rule_arch == native_arch ||
++		    cur_rule_arch == lxc_seccomp_arch_native ||
++		    compat_arch == SCMP_ARCH_NATIVE) {
++			INFO("Adding native rule for %s action %d", line, action);
+ 			if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line, conf->seccomp_ctx, action))
+ 				goto bad_rule;
+ 		}
+-
+-		/*
+-		 * TODO generalize - if need_compat(native_arch, cur_rule_arch)
+-		 */
+-		if (native_arch == lxc_seccomp_arch_amd64 &&
+-			cur_rule_arch != lxc_seccomp_arch_amd64) {
+-			int nr1, nr2;
++		else if (cur_rule_arch != lxc_seccomp_arch_all) {
++			INFO("Adding compat-only rule for %s action %d", line, action);
++			if (!do_resolve_add_rule(compat_arch, line, compat_ctx, action))
++				goto bad_rule;
++		}
++		else {
++			INFO("Adding native rule for %s action %d", line, action);
++			if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line, conf->seccomp_ctx, action))
++				goto bad_rule;
+ 			INFO("Adding compat rule for %s action %d", line, action);
+-			nr1 = seccomp_syscall_resolve_name_arch(SCMP_ARCH_X86, line);
+-			nr2 = seccomp_syscall_resolve_name_arch(SCMP_ARCH_NATIVE, line);
+-			if (nr1 == nr2) {
+-				/* If the syscall # is the same for 32- and 64-bit, then we cannot
+-				 * apply it to the compat_ctx.  So apply it to the noncompat ctx.
+-				 * We may already have done so, but that's ok
+-				 */
+-				INFO("Adding non-compat rule bc nr1 == nr2 (%d, %d)", nr1, nr2);
+-				if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line, conf->seccomp_ctx, action))
+-					goto bad_rule;
+-				continue;
+-			}
+-			INFO("Really adding compat rule bc nr1 == nr2 (%d, %d)", nr1, nr2);
+-			if (!do_resolve_add_rule(SCMP_ARCH_X86, line,
+-						compat_ctx, action))
++			if (!do_resolve_add_rule(compat_arch, line, compat_ctx, action))
+ 				goto bad_rule;
+ 		}
+ 	}
+-- 
+2.1.4
+
diff --git a/debian/patches/series b/debian/patches/series
index 5c40332..3abfda1 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -6,3 +6,5 @@ do-dot-call-chown_mapped_root-on-etc-pve.patch
 use-var-lib-vz-as-default-dir.patch
 do-not-use-config-path-for-rootfs.patch
 run-lxcnetaddbr.patch
+0001-seccomp-simplify-and-fix-rule-parsing.patch
+0001-pass-on-reboot-flag-and-delete-old-veth-on-reboot.patch
-- 
2.1.4




More information about the pve-devel mailing list