[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