[lxc-devel] [PATCH 1/1] seccomp: fix 32-bit rules
Stéphane Graber
stgraber at ubuntu.com
Fri Jun 20 20:33:27 UTC 2014
On Fri, Jun 20, 2014 at 02:58:41PM -0500, Serge Hallyn wrote:
> 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 <serge.hallyn at ubuntu.com>
Acked-by: Stéphane Graber <stgraber at ubuntu.com>
> ---
> src/lxc/seccomp.c | 72 ++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 40 insertions(+), 32 deletions(-)
>
> diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
> index 5df37bc..dfdedf2 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
> --
> 2.0.0
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140620/15880ba2/attachment-0001.sig>
More information about the lxc-devel
mailing list