From: Serge Hallyn Date: Fri, 20 Jun 2014 19:58:41 +0000 (-0500) Subject: seccomp: fix 32-bit rules X-Git-Tag: lxc-1.1.0.alpha1~31 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cd75548b25f39b4ee36dc20e70c8e1b379a287f8;p=thirdparty%2Flxc.git seccomp: fix 32-bit rules When calling seccomp_rule_add(), you must pass the native syscall number even if the context is a 32-bit context. So use resolve_name rather than resolve_name_arch. Enhance the check of /proc/self/status for Seccomp: so that we do not enable seccomp policies if seccomp is not built into the kernel. This is needed before we can enable by-default seccomp policies (which we want to do next) Fix wrong return value check from seccomp_arch_exist, and remove needless abstraction in arch handling. Signed-off-by: Serge Hallyn Acked-by: Stéphane Graber --- diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c index 5df37bc6f..dfdedf22b 100644 --- a/src/lxc/seccomp.c +++ b/src/lxc/seccomp.c @@ -182,11 +182,11 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx, { int nr, ret; - if (arch && !seccomp_arch_exist(ctx, arch)) { + if (arch && seccomp_arch_exist(ctx, arch) != 0) { ERROR("BUG: seccomp: rule and context arch do not match (arch %d)", arch); return false; } - nr = seccomp_syscall_resolve_name_arch(arch, line); + nr = seccomp_syscall_resolve_name(line); if (nr == __NR_SCMP_ERROR) { WARN("Seccomp: failed to resolve syscall: %s", line); WARN("This syscall will NOT be blacklisted"); @@ -226,7 +226,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) scmp_filter_ctx compat_ctx = NULL; bool blacklist = false; uint32_t default_policy_action = -1, default_rule_action = -1, action; - uint32_t arch = SCMP_ARCH_NATIVE; enum lxc_hostarch_t native_arch = get_hostarch(), cur_rule_arch = native_arch; @@ -294,7 +293,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) continue; } cur_rule_arch = lxc_seccomp_arch_i386; - arch = SCMP_ARCH_X86; if (native_arch == lxc_seccomp_arch_amd64) { if (compat_ctx) continue; @@ -310,7 +308,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) continue; } cur_rule_arch = lxc_seccomp_arch_amd64; - arch = SCMP_ARCH_X86_64; } else if (strcmp(line, "[all]") == 0 || strcmp(line, "[ALL]") == 0) { cur_rule_arch = lxc_seccomp_arch_all; @@ -331,7 +328,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) continue; } cur_rule_arch = lxc_seccomp_arch_arm; - arch = SCMP_ARCH_ARM; } #endif else @@ -351,16 +347,16 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) goto bad_rule; } - if (cur_rule_arch == lxc_seccomp_arch_all) - arch = SCMP_ARCH_NATIVE; - /* * 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 (!do_resolve_add_rule(arch, line, conf->seccomp_ctx, action)) + if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line, conf->seccomp_ctx, action)) goto bad_rule; } @@ -371,17 +367,19 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) cur_rule_arch != lxc_seccomp_arch_amd64) { int nr1, nr2; INFO("Adding compat rule for %s action %d", line, action); - nr1 = seccomp_syscall_resolve_name_arch(lxc_seccomp_arch_amd64, line); - nr2 = seccomp_syscall_resolve_name_arch(lxc_seccomp_arch_i386, line); + nr1 = seccomp_syscall_resolve_name_arch(SCMP_ARCH_X86, line); + nr2 = seccomp_syscall_resolve_name_arch(SCMP_ARCH_NATIVE, line); if (nr1 == nr2) { - /* sigh - if the syscall # is the same for 32- and 64-bit, then - * we get EFAULT if we try to aplly to compat_ctx. So apply to - * the noncompat ctx. We may already have done so, but that's ok + /* 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 */ - if (!do_resolve_add_rule(arch, line, conf->seccomp_ctx, action)) + 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)) goto bad_rule; @@ -449,30 +447,44 @@ static int parse_config(FILE *f, struct lxc_conf *conf) return parse_config_v2(f, line, conf); } -static bool seccomp_already_confined(void) +/* + * use_seccomp: return true if we should try and apply a seccomp policy + * if defined for the container. + * This will return false if + * 1. seccomp is not enabled in the kernel + * 2. a seccomp policy is already enabled for this task + */ +static bool use_seccomp(void) { FILE *f = fopen("/proc/self/status", "r"); char line[1024]; - bool bret = false; + bool already_enabled = false; + bool found = false; int ret, v; if (!f) - return false; + return true; while (fgets(line, 1024, f)) { if (strncmp(line, "Seccomp:", 8) == 0) { + found = true; ret = sscanf(line+8, "%d", &v); - if (ret != 1) - continue; - if (v != 0) - bret = true; - goto out; + if (ret == 1 && v != 0) + already_enabled = true; + break; } } -out: fclose(f); - return bret; + if (!found) { /* no Seccomp line, no seccomp in kernel */ + INFO("Seccomp is not enabled in the kernel"); + return false; + } + if (already_enabled) { /* already seccomp-confined */ + INFO("Already seccomp-confined, not loading new policy"); + return false; + } + return true; } int lxc_read_seccomp_config(struct lxc_conf *conf) @@ -483,10 +495,8 @@ int lxc_read_seccomp_config(struct lxc_conf *conf) if (!conf->seccomp) return 0; - if (seccomp_already_confined()) { - INFO("Already confined by seccomp; not loading a new policy"); + if (!use_seccomp()) return 0; - } #if HAVE_SCMP_FILTER_CTX /* XXX for debug, pass in SCMP_ACT_TRAP */ conf->seccomp_ctx = seccomp_init(SCMP_ACT_KILL); @@ -525,10 +535,8 @@ int lxc_seccomp_load(struct lxc_conf *conf) int ret; if (!conf->seccomp) return 0; - if (seccomp_already_confined()) { - INFO("Already confined by seccomp; not loading a new policy"); + if (!use_seccomp()) return 0; - } ret = seccomp_load( #if HAVE_SCMP_FILTER_CTX conf->seccomp_ctx