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 <vapier@gentoo.org>
Signed-off-by: Paul Moore <pmoore@redhat.com>

12 files changed, 193 insertions, 46 deletions

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 <pmoore@redhat.com>
+ * Author: Paul Moore <pmoore@redhat.com>
+ */
+
+/*
+ * 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 <http://www.gnu.org/licenses>.
+ */
+
+#include <errno.h>
+#include <unistd.h>
+
+#include <seccomp.h>
+
+#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 <pmoore@redhat.com>
+# Author: Paul Moore <pmoore@redhat.com>
+#
+
+#
+# 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 <http://www.gnu.org/licenses>.
+#
+
+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 <pmoore@redhat.com>
+# Author: Paul Moore <pmoore@redhat.com
+#
+
+test type: bpf-sim
+
+# Testname		Arch	Syscall		Arg0	Arg1	Arg2	Arg3	Arg4	Arg5	Result
+29-sim-pseudo_syscall	+x86	0-10		N	N	N	N	N	N	ALLOW
+29-sim-pseudo_syscall	+x86	4294957190	N	N	N	N	N	N	ALLOW
+29-sim-pseudo_syscall	+x86	4294957295	N	N	N	N	N	N	ALLOW
+
+test type: bpf-sim-fuzz
+
+# Testname		StressCount
+29-sim-pseudo_syscall	50
+
+test type: bpf-valgrind
+
+# Testname
+29-sim-pseudo_syscall
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ba670c6..71e18ec 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -56,7 +56,8 @@ check_PROGRAMS = \
 	25-sim-multilevel_chains_adv \
 	26-sim-arch_all_be_basic \
 	27-sim-bpf_blk_state \
-	28-sim-arch_x86
+	28-sim-arch_x86 \
+	29-sim-pseudo_syscall
 
 EXTRA_DIST_TESTPYTHON = \
 	util.py \
@@ -87,7 +88,8 @@ EXTRA_DIST_TESTPYTHON = \
 	25-sim-multilevel_chains_adv.py \
 	26-sim-arch_all_be_basic.py \
 	27-sim-bpf_blk_state.py \
-	28-sim-arch_x86.py
+	28-sim-arch_x86.py \
+	29-sim-pseudo_syscall.py
 
 EXTRA_DIST_TESTCFGS = \
 	01-sim-allow.tests \
@@ -117,7 +119,8 @@ EXTRA_DIST_TESTCFGS = \
 	25-sim-multilevel_chains_adv.tests \
 	26-sim-arch_all_be_basic.tests \
 	27-sim-bpf_blk_state.tests \
-	28-sim-arch_x86.tests
+	28-sim-arch_x86.tests \
+	29-sim-pseudo_syscall.tests
 
 EXTRA_DIST_TESTSCRIPTS = regression testdiff testgen