From 51c46f80c1edee863bbc4eb21b03decc44e69a45 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 24 Aug 2015 18:05:05 -0400 Subject: all: block negative syscall numbers from the filter We use negative syscalls numbers to indicate syscalls that aren't supported by a certain arch/ABI and unfortunately there were cases where these bogus syscall values were finding their way into the filter. This patch corrects this and adds a new test to check for this in the future. Reported-by: Mike Frysinger Signed-off-by: Paul Moore --- src/api.c | 9 +++-- src/arch-x86.c | 29 ++++++++-------- src/arch-x86.h | 2 +- src/arch.c | 30 ++++++++--------- src/arch.h | 3 +- src/db.c | 5 ++- src/db.h | 5 ++- tests/.gitignore | 2 +- tests/29-sim-pseudo_syscall.c | 71 +++++++++++++++++++++++++++++++++++++++ tests/29-sim-pseudo_syscall.py | 51 ++++++++++++++++++++++++++++ tests/29-sim-pseudo_syscall.tests | 23 +++++++++++++ tests/Makefile.am | 9 +++-- 12 files changed, 193 insertions(+), 46 deletions(-) create mode 100644 tests/29-sim-pseudo_syscall.c create mode 100644 tests/29-sim-pseudo_syscall.py create mode 100644 tests/29-sim-pseudo_syscall.tests diff --git a/src/api.c b/src/api.c index a16d4c2..0cfa407 100644 --- a/src/api.c +++ b/src/api.c @@ -298,6 +298,7 @@ API int seccomp_syscall_resolve_name_arch(uint32_t arch_token, const char *name) API int seccomp_syscall_resolve_name_rewrite(uint32_t arch_token, const char *name) { + int rc; int syscall; const struct arch_def *arch; @@ -315,7 +316,11 @@ API int seccomp_syscall_resolve_name_rewrite(uint32_t arch_token, syscall = arch_syscall_resolve_name(arch, name); if (syscall == __NR_SCMP_ERROR) return __NR_SCMP_ERROR; - if (arch_syscall_rewrite(arch, 0, &syscall) < 0) + rc = arch_syscall_rewrite(arch, &syscall); + if (rc == -EDOM) + /* if we can't rewrite the syscall, just pass it through */ + return syscall; + else if (rc < 0) return __NR_SCMP_ERROR; return syscall; @@ -356,7 +361,7 @@ API int seccomp_syscall_priority(scmp_filter_ctx ctx, * since priorities are a "best effort" thing - as we * want to catch the -EDOM error and bail on this * architecture */ - rc_tmp = arch_syscall_rewrite(filter->arch, 1, &sc_tmp); + rc_tmp = arch_syscall_rewrite(filter->arch, &sc_tmp); if (rc_tmp == -EDOM) continue; if (rc_tmp < 0) diff --git a/src/arch-x86.c b/src/arch-x86.c index 1b4f2a9..a08ffdf 100644 --- a/src/arch-x86.c +++ b/src/arch-x86.c @@ -40,24 +40,22 @@ const struct arch_def arch_def_x86 = { /** * Rewrite a syscall value to match the architecture * @param arch the architecture definition - * @param strict strict flag * @param syscall the syscall number * * Syscalls can vary across different architectures so this function rewrites - * the syscall into the correct value for the specified architecture. If - * @strict is true then the function will fail if the syscall can not be - * preservered, however, if @strict is false the function will do a "best - * effort" rewrite and not fail. Returns zero on success, negative values on - * failure. + * the syscall into the correct value for the specified architecture. Returns + * zero on success, negative values on failure. * */ -int x86_syscall_rewrite(const struct arch_def *arch, bool strict, int *syscall) +int x86_syscall_rewrite(const struct arch_def *arch, int *syscall) { - if ((*syscall) <= -100 && (*syscall) >= -117) + int sys = *syscall; + + if (sys <= -100 && sys >= -117) *syscall = __x86_NR_socketcall; - else if ((*syscall) <= -200 && (*syscall) >= -211) + else if (sys <= -200 && sys >= -211) *syscall = __x86_NR_ipc; - else if (((*syscall) < 0) && (strict)) + else if (sys < 0) return -EDOM; return 0; @@ -81,6 +79,7 @@ int x86_syscall_rewrite(const struct arch_def *arch, bool strict, int *syscall) int x86_filter_rewrite(const struct arch_def *arch, bool strict, int *syscall, struct db_api_arg *chain) { + int sys = *syscall; unsigned int iter; int arg_max; @@ -88,7 +87,7 @@ int x86_filter_rewrite(const struct arch_def *arch, bool strict, if (arg_max < 0) return arg_max; - if ((*syscall) <= -100 && (*syscall) >= -117) { + if (sys <= -100 && sys >= -117) { for (iter = 0; iter < arg_max; iter++) { if ((chain[iter].valid != 0) && (strict)) return -EINVAL; @@ -96,10 +95,10 @@ int x86_filter_rewrite(const struct arch_def *arch, bool strict, chain[0].arg = 0; chain[0].op = SCMP_CMP_EQ; chain[0].mask = DATUM_MAX; - chain[0].datum = abs(*syscall) % 100; + chain[0].datum = abs(sys) % 100; chain[0].valid = 1; *syscall = __x86_NR_socketcall; - } else if ((*syscall) <= -200 && (*syscall) >= -211) { + } else if (sys <= -200 && sys >= -211) { for (iter = 0; iter < arg_max; iter++) { if ((chain[iter].valid != 0) && (strict)) return -EINVAL; @@ -107,10 +106,10 @@ int x86_filter_rewrite(const struct arch_def *arch, bool strict, chain[0].arg = 0; chain[0].op = SCMP_CMP_EQ; chain[0].mask = DATUM_MAX; - chain[0].datum = abs(*syscall) % 200; + chain[0].datum = abs(sys) % 200; chain[0].valid = 1; *syscall = __x86_NR_ipc; - } else if (((*syscall) < 0) && (strict)) + } else if (sys < 0) return -EDOM; return 0; diff --git a/src/arch-x86.h b/src/arch-x86.h index 163d0ed..6190519 100644 --- a/src/arch-x86.h +++ b/src/arch-x86.h @@ -35,7 +35,7 @@ const char *x86_syscall_resolve_num(int num); const char *x86_syscall_iterate_name(unsigned int spot); -int x86_syscall_rewrite(const struct arch_def *arch, bool strict, int *syscall); +int x86_syscall_rewrite(const struct arch_def *arch, int *syscall); int x86_filter_rewrite(const struct arch_def *arch, bool strict, int *syscall, struct db_api_arg *chain); diff --git a/src/arch.c b/src/arch.c index 25d1ff6..5bb7f69 100644 --- a/src/arch.c +++ b/src/arch.c @@ -383,18 +383,15 @@ int arch_syscall_translate(const struct arch_def *arch, int *syscall) /** * Rewrite a syscall value to match the architecture * @param arch the architecture definition - * @param strict strict flag * @param syscall the syscall number * * Syscalls can vary across different architectures so this function rewrites - * the syscall into the correct value for the specified architecture. If - * @strict is true then the function will fail if the syscall can not be - * preservered, however, if @strict is false the function will do a "best - * effort" rewrite and not fail. Returns zero on success, negative values on - * failure. + * the syscall into the correct value for the specified architecture. Returns + * zero on success, -EDOM if the syscall is not defined for @arch, and negative + * values on failure. * */ -int arch_syscall_rewrite(const struct arch_def *arch, bool strict, int *syscall) +int arch_syscall_rewrite(const struct arch_def *arch, int *syscall) { int sys = *syscall; @@ -408,14 +405,12 @@ int arch_syscall_rewrite(const struct arch_def *arch, bool strict, int *syscall) /* rewritable syscalls */ switch (arch->token) { case SCMP_ARCH_X86: - return x86_syscall_rewrite(arch, strict, syscall); + x86_syscall_rewrite(arch, syscall); } - /* NOTE: we fall through to the default handling (strict?) if - * we don't support any rewriting for the architecture */ } /* syscalls not defined on this architecture */ - if (strict) + if ((*syscall) < 0) return -EDOM; return 0; } @@ -432,12 +427,14 @@ int arch_syscall_rewrite(const struct arch_def *arch, bool strict, int *syscall) * regardless of the rule or architecture. If @strict is true then the * function will fail if the entire filter can not be preservered, however, * if @strict is false the function will do a "best effort" rewrite and not - * fail. Returns zero on success, negative values on failure. + * fail. Returns zero on success, -EDOM if the syscall is not defined for + * @arch, and negative values on failure. * */ int arch_filter_rewrite(const struct arch_def *arch, bool strict, int *syscall, struct db_api_arg *chain) { + int rc; int sys = *syscall; if (sys >= 0) { @@ -450,14 +447,15 @@ int arch_filter_rewrite(const struct arch_def *arch, /* rewritable syscalls */ switch (arch->token) { case SCMP_ARCH_X86: - return x86_filter_rewrite(arch, strict, syscall, chain); + rc = x86_filter_rewrite(arch, strict, syscall, chain); + /* we still want to catch invalid rewrites */ + if (rc == -EINVAL) + return -EINVAL; } - /* NOTE: we fall through to the default handling (strict?) if - * we don't support any rewriting for the architecture */ } /* syscalls not defined on this architecture */ - if (strict) + if ((*syscall) < 0) return -EDOM; return 0; } diff --git a/src/arch.h b/src/arch.h index aa3158c..84e54e5 100644 --- a/src/arch.h +++ b/src/arch.h @@ -90,8 +90,7 @@ int arch_syscall_resolve_name(const struct arch_def *arch, const char *name); const char *arch_syscall_resolve_num(const struct arch_def *arch, int num); int arch_syscall_translate(const struct arch_def *arch, int *syscall); -int arch_syscall_rewrite(const struct arch_def *arch, bool strict, - int *syscall); +int arch_syscall_rewrite(const struct arch_def *arch, int *syscall); int arch_filter_rewrite(const struct arch_def *arch, bool strict, int *syscall, struct db_api_arg *chain); diff --git a/src/db.c b/src/db.c index 604d058..37b334a 100644 --- a/src/db.c +++ b/src/db.c @@ -780,8 +780,7 @@ void db_release(struct db_filter *db) * negative values on failure. * */ -int db_syscall_priority(struct db_filter *db, - unsigned int syscall, uint8_t priority) +int db_syscall_priority(struct db_filter *db, int syscall, uint8_t priority) { unsigned int sys_pri = _DB_PRI_USER(priority); struct db_sys_list *s_new, *s_iter, *s_prev = NULL; @@ -1128,7 +1127,7 @@ gen_32_failure: * filter DB. Returns zero on success, negative values on failure. * */ -int db_rule_add(struct db_filter *db, uint32_t action, unsigned int syscall, +int db_rule_add(struct db_filter *db, uint32_t action, int syscall, struct db_api_arg *chain) { int rc = -ENOMEM; diff --git a/src/db.h b/src/db.h index 376cbc3..bfd9333 100644 --- a/src/db.h +++ b/src/db.h @@ -188,10 +188,9 @@ struct db_filter *db_init(const struct arch_def *arch); void db_reset(struct db_filter *db); void db_release(struct db_filter *db); -int db_syscall_priority(struct db_filter *db, - unsigned int syscall, uint8_t priority); +int db_syscall_priority(struct db_filter *db, int syscall, uint8_t priority); -int db_rule_add(struct db_filter *db, uint32_t action, unsigned int syscall, +int db_rule_add(struct db_filter *db, uint32_t action, int syscall, struct db_api_arg *chain); #endif diff --git a/tests/.gitignore b/tests/.gitignore index d47ea65..e0297ea 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -33,4 +33,4 @@ util.pyc 26-sim-arch_all_be_basic 27-sim-bpf_blk_state 28-sim-arch_x86 - +29-sim-pseudo_syscall diff --git a/tests/29-sim-pseudo_syscall.c b/tests/29-sim-pseudo_syscall.c new file mode 100644 index 0000000..d909fd8 --- /dev/null +++ b/tests/29-sim-pseudo_syscall.c @@ -0,0 +1,71 @@ +/** + * Seccomp Library test program + * + * Copyright (c) 2015 Red Hat + * Author: Paul Moore + */ + +/* + * This library is free software; you can redistribute it and/or modify it + * under the terms of version 2.1 of the GNU Lesser General Public License as + * published by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, see . + */ + +#include +#include + +#include + +#include "util.h" + +int main(int argc, char *argv[]) +{ + int rc; + struct util_options opts; + scmp_filter_ctx ctx = NULL; + + rc = util_getopt(argc, argv, &opts); + if (rc < 0) + goto out; + + ctx = seccomp_init(SCMP_ACT_ALLOW); + if (ctx == NULL) + return ENOMEM; + + /* NOTE: we have to be careful here because some ABIs use syscall + * offsets which could interfere with our test, x86 is safe */ + rc = seccomp_arch_remove(ctx, SCMP_ARCH_NATIVE); + if (rc < 0) + goto out; + rc = seccomp_arch_add(ctx, SCMP_ARCH_X86); + if (rc < 0) + goto out; + + /* SCMP_SYS(sysmips) == 4294957190 (unsigned) */ + rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(sysmips), 0); + if (rc < 0) + goto out; + rc = seccomp_rule_add_exact(ctx, SCMP_ACT_KILL, SCMP_SYS(sysmips), 0); + if (rc == 0) + goto out; + /* -10001 == 4294957295 (unsigned) */ + rc = seccomp_rule_add_exact(ctx, SCMP_ACT_KILL, -11001, 0); + if (rc == 0) + goto out; + + rc = util_filter_output(&opts, ctx); + if (rc) + goto out; + +out: + seccomp_release(ctx); + return (rc < 0 ? -rc : rc); +} diff --git a/tests/29-sim-pseudo_syscall.py b/tests/29-sim-pseudo_syscall.py new file mode 100644 index 0000000..3d39eaf --- /dev/null +++ b/tests/29-sim-pseudo_syscall.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python + +# +# Seccomp Library test program +# +# Copyright (c) 2015 Red Hat +# Author: Paul Moore +# + +# +# This library is free software; you can redistribute it and/or modify it +# under the terms of version 2.1 of the GNU Lesser General Public License as +# published by the Free Software Foundation. +# +# This library is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License +# for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library; if not, see . +# + +import argparse +import sys + +import util + +from seccomp import * + +def test(args): + f = SyscallFilter(ALLOW) + f.remove_arch(Arch()) + f.add_arch(Arch("x86")) + f.add_rule(KILL, "sysmips") + try: + f.add_rule_exactly(KILL, "sysmips") + except RuntimeError: + pass + try: + f.add_rule_exactly(KILL, -10001) + except RuntimeError: + pass + return f + +args = util.get_opt() +ctx = test(args) +util.filter_output(args, ctx) + +# kate: syntax python; +# kate: indent-mode python; space-indent on; indent-width 4; mixedindent off; diff --git a/tests/29-sim-pseudo_syscall.tests b/tests/29-sim-pseudo_syscall.tests new file mode 100644 index 0000000..028b858 --- /dev/null +++ b/tests/29-sim-pseudo_syscall.tests @@ -0,0 +1,23 @@ +# +# libseccomp regression test automation data +# +# Copyright (c) 2015 Red Hat +# Author: Paul Moore