Commit 0995cb73 by Serge Hallyn Committed by Stéphane Graber

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: 's avatarSerge Hallyn <serge.hallyn@ubuntu.com> Acked-by: 's avatarStéphane Graber <stgraber@ubuntu.com>
parent 391ece78
...@@ -182,11 +182,11 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx, ...@@ -182,11 +182,11 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
{ {
int nr, ret; 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); ERROR("BUG: seccomp: rule and context arch do not match (arch %d)", arch);
return false; return false;
} }
nr = seccomp_syscall_resolve_name_arch(arch, line); nr = seccomp_syscall_resolve_name(line);
if (nr == __NR_SCMP_ERROR) { if (nr == __NR_SCMP_ERROR) {
WARN("Seccomp: failed to resolve syscall: %s", line); WARN("Seccomp: failed to resolve syscall: %s", line);
WARN("This syscall will NOT be blacklisted"); WARN("This syscall will NOT be blacklisted");
...@@ -226,7 +226,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) ...@@ -226,7 +226,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
scmp_filter_ctx compat_ctx = NULL; scmp_filter_ctx compat_ctx = NULL;
bool blacklist = false; bool blacklist = false;
uint32_t default_policy_action = -1, default_rule_action = -1, action; 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(), enum lxc_hostarch_t native_arch = get_hostarch(),
cur_rule_arch = native_arch; cur_rule_arch = native_arch;
...@@ -294,7 +293,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) ...@@ -294,7 +293,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
continue; continue;
} }
cur_rule_arch = lxc_seccomp_arch_i386; cur_rule_arch = lxc_seccomp_arch_i386;
arch = SCMP_ARCH_X86;
if (native_arch == lxc_seccomp_arch_amd64) { if (native_arch == lxc_seccomp_arch_amd64) {
if (compat_ctx) if (compat_ctx)
continue; continue;
...@@ -310,7 +308,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) ...@@ -310,7 +308,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
continue; continue;
} }
cur_rule_arch = lxc_seccomp_arch_amd64; cur_rule_arch = lxc_seccomp_arch_amd64;
arch = SCMP_ARCH_X86_64;
} else if (strcmp(line, "[all]") == 0 || } else if (strcmp(line, "[all]") == 0 ||
strcmp(line, "[ALL]") == 0) { strcmp(line, "[ALL]") == 0) {
cur_rule_arch = lxc_seccomp_arch_all; cur_rule_arch = lxc_seccomp_arch_all;
...@@ -331,7 +328,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) ...@@ -331,7 +328,6 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
continue; continue;
} }
cur_rule_arch = lxc_seccomp_arch_arm; cur_rule_arch = lxc_seccomp_arch_arm;
arch = SCMP_ARCH_ARM;
} }
#endif #endif
else else
...@@ -351,16 +347,16 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) ...@@ -351,16 +347,16 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
goto bad_rule; 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) * 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 && if (!(cur_rule_arch == lxc_seccomp_arch_i386 &&
native_arch == lxc_seccomp_arch_amd64)) { native_arch == lxc_seccomp_arch_amd64)) {
INFO("Adding non-compat rule for %s action %d", line, action); 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; goto bad_rule;
} }
...@@ -371,17 +367,19 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf) ...@@ -371,17 +367,19 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
cur_rule_arch != lxc_seccomp_arch_amd64) { cur_rule_arch != lxc_seccomp_arch_amd64) {
int nr1, nr2; int nr1, nr2;
INFO("Adding compat rule for %s action %d", line, action); INFO("Adding compat rule for %s action %d", line, action);
nr1 = seccomp_syscall_resolve_name_arch(lxc_seccomp_arch_amd64, line); nr1 = seccomp_syscall_resolve_name_arch(SCMP_ARCH_X86, line);
nr2 = seccomp_syscall_resolve_name_arch(lxc_seccomp_arch_i386, line); nr2 = seccomp_syscall_resolve_name_arch(SCMP_ARCH_NATIVE, line);
if (nr1 == nr2) { if (nr1 == nr2) {
/* sigh - if the syscall # is the same for 32- and 64-bit, then /* If the syscall # is the same for 32- and 64-bit, then we cannot
* we get EFAULT if we try to aplly to compat_ctx. So apply to * apply it to the compat_ctx. So apply it to the noncompat ctx.
* the noncompat ctx. We may already have done so, but that's ok * 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; goto bad_rule;
continue; continue;
} }
INFO("Really adding compat rule bc nr1 == nr2 (%d, %d)", nr1, nr2);
if (!do_resolve_add_rule(SCMP_ARCH_X86, line, if (!do_resolve_add_rule(SCMP_ARCH_X86, line,
compat_ctx, action)) compat_ctx, action))
goto bad_rule; goto bad_rule;
...@@ -449,30 +447,44 @@ static int parse_config(FILE *f, struct lxc_conf *conf) ...@@ -449,30 +447,44 @@ static int parse_config(FILE *f, struct lxc_conf *conf)
return parse_config_v2(f, line, 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"); FILE *f = fopen("/proc/self/status", "r");
char line[1024]; char line[1024];
bool bret = false; bool already_enabled = false;
bool found = false;
int ret, v; int ret, v;
if (!f) if (!f)
return false; return true;
while (fgets(line, 1024, f)) { while (fgets(line, 1024, f)) {
if (strncmp(line, "Seccomp:", 8) == 0) { if (strncmp(line, "Seccomp:", 8) == 0) {
found = true;
ret = sscanf(line+8, "%d", &v); ret = sscanf(line+8, "%d", &v);
if (ret != 1) if (ret == 1 && v != 0)
continue; already_enabled = true;
if (v != 0) break;
bret = true;
goto out;
} }
} }
out:
fclose(f); 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) int lxc_read_seccomp_config(struct lxc_conf *conf)
...@@ -483,10 +495,8 @@ 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) if (!conf->seccomp)
return 0; return 0;
if (seccomp_already_confined()) { if (!use_seccomp())
INFO("Already confined by seccomp; not loading a new policy");
return 0; return 0;
}
#if HAVE_SCMP_FILTER_CTX #if HAVE_SCMP_FILTER_CTX
/* XXX for debug, pass in SCMP_ACT_TRAP */ /* XXX for debug, pass in SCMP_ACT_TRAP */
conf->seccomp_ctx = seccomp_init(SCMP_ACT_KILL); conf->seccomp_ctx = seccomp_init(SCMP_ACT_KILL);
...@@ -525,10 +535,8 @@ int lxc_seccomp_load(struct lxc_conf *conf) ...@@ -525,10 +535,8 @@ int lxc_seccomp_load(struct lxc_conf *conf)
int ret; int ret;
if (!conf->seccomp) if (!conf->seccomp)
return 0; return 0;
if (seccomp_already_confined()) { if (!use_seccomp())
INFO("Already confined by seccomp; not loading a new policy");
return 0; return 0;
}
ret = seccomp_load( ret = seccomp_load(
#if HAVE_SCMP_FILTER_CTX #if HAVE_SCMP_FILTER_CTX
conf->seccomp_ctx conf->seccomp_ctx
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment