api: provide 32-bit friendly argument comparison macros

We have a longstanding issue with 32-bit to 64-bit sign extension
inadvertently resulting in bogus syscall argument extensions. This
patch introduces a new set of argument comparison macros which
limit the argument values to 32-bit values so that we don't run into
problems with sign extension.

We use the macro overloading proposed by Roman at
https://kecher.net/overloading-macros/ to retain the feature of these
macros being usable as static initializers.

Thanks to @jdstrand on GitHub for reporting the problem.

Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Michael Weiser <michael.weiser@gmx.de>

6 files changed, 295 insertions, 22 deletions

diff --git a/doc/man/man3/seccomp_rule_add.3 b/doc/man/man3/seccomp_rule_add.3
index b051577..e86f41b 100644
--- a/doc/man/man3/seccomp_rule_add.3
+++ b/doc/man/man3/seccomp_rule_add.3
@@ -1,4 +1,4 @@
-.TH "seccomp_rule_add" 3 "25 July 2012" "paul@paul-moore.com" "libseccomp Documentation"
+.TH "seccomp_rule_add" 3 "17 February 2019" "paul@paul-moore.com" "libseccomp Documentation"
 .\" //////////////////////////////////////////////////////////////////////////
 .SH NAME
 .\" //////////////////////////////////////////////////////////////////////////
@@ -22,6 +22,24 @@ seccomp_rule_add, seccomp_rule_add_exact \- Add a seccomp filter rule
 .BI "struct scmp_arg_cmp SCMP_A4(enum scmp_compare " op ", " ... ");"
 .BI "struct scmp_arg_cmp SCMP_A5(enum scmp_compare " op ", " ... ");"
 .sp
+.BI "struct scmp_arg_cmp SCMP_CMP64(unsigned int " arg ","
+.BI "                             enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A0_64(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A1_64(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A2_64(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A3_64(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A4_64(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A5_64(enum scmp_compare " op ", " ... ");"
+.sp
+.BI "struct scmp_arg_cmp SCMP_CMP32(unsigned int " arg ","
+.BI "                             enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A0_32(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A1_32(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A2_32(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A3_32(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A4_32(enum scmp_compare " op ", " ... ");"
+.BI "struct scmp_arg_cmp SCMP_A5_32(enum scmp_compare " op ", " ... ");"
+.sp
 .BI "int seccomp_rule_add(scmp_filter_ctx " ctx ", uint32_t " action ","
 .BI "                     int " syscall ", unsigned int " arg_cnt ", " ... ");"
 .BI "int seccomp_rule_add_exact(scmp_filter_ctx " ctx ", uint32_t " action ","
@@ -71,15 +89,36 @@ loaded into the kernel using
 .BR seccomp_load (3).
 .P
 The
+.BR SCMP_CMP (),
+.BR SCMP_CMP64 (),
+.BR SCMP_A{0-5} (),
+and
+.BR SCMP_A{0-5}_64 ()
+macros generate a scmp_arg_cmp structure for use with the above functions. The
 .BR SCMP_CMP ()
 and
+.BR SCMP_CMP64 ()
+macros allows the caller to specify an arbitrary argument along with the
+comparison operator, 64-bit mask, and 64-bit datum values where the
 .BR SCMP_A{0-5} ()
-macros generate a scmp_arg_cmp structure for use with the above functions. The
+and
+.BR SCMP_A{0-5}_64 ()
+macros are specific to a certain argument.
+.P
+The
+.BR SCMP_CMP32 ()
+and
+.BR SCMP_A{0-5}_32 ()
+macros are similar to the variants above, but they take 32-bit mask and 32-bit
+datum values.
+.P
+It is recommended that whenever possible developers avoid using the
 .BR SCMP_CMP ()
-macro allows the caller to specify an arbitrary argument along with the
-comparison operator, mask, and datum values where the
+and
 .BR SCMP_A{0-5} ()
-macros are specific to a certain argument.  See the EXAMPLES section below.
+macros and use the variants which are explicitly 32 or 64-bit.  This should
+help eliminate problems caused by an unwanted sign extension of negative datum
+values.
 .P
 While it is possible to specify the
 .I syscall
diff --git a/include/seccomp.h.in b/include/seccomp.h.in
index 20ce45e..b28e76e 100644
--- a/include/seccomp.h.in
+++ b/include/seccomp.h.in
@@ -198,44 +198,103 @@ struct scmp_arg_cmp {
  */
 #define SCMP_SYS(x)		(__NR_##x)
 
+/* Helpers for the argument comparison macros, DO NOT USE directly */
+#define _SCMP_VA_NUM_ARGS(...)	_SCMP_VA_NUM_ARGS_IMPL(__VA_ARGS__,2,1)
+#define _SCMP_VA_NUM_ARGS_IMPL(_1,_2,N,...)	N
+#define _SCMP_MACRO_DISPATCHER(func, ...) \
+	_SCMP_MACRO_DISPATCHER_IMPL1(func, _SCMP_VA_NUM_ARGS(__VA_ARGS__))
+#define _SCMP_MACRO_DISPATCHER_IMPL1(func, nargs) \
+	_SCMP_MACRO_DISPATCHER_IMPL2(func, nargs)
+#define _SCMP_MACRO_DISPATCHER_IMPL2(func, nargs) \
+	func ## nargs
+#define _SCMP_CMP32_1(x, y, z)		SCMP_CMP64(x, y, (uint32_t)(z))
+#define _SCMP_CMP32_2(x, y, z, q)	SCMP_CMP64(x, y, (uint32_t)(z), (uint32_t)(q))
+
 /**
- * Specify an argument comparison struct for use in declaring rules
+ * Specify a 64-bit argument comparison struct for use in declaring rules
  * @param arg the argument number, starting at 0
  * @param op the comparison operator, e.g. SCMP_CMP_*
  * @param datum_a dependent on comparison
  * @param datum_b dependent on comparison, optional
  */
-#define SCMP_CMP(...)		((struct scmp_arg_cmp){__VA_ARGS__})
+#define SCMP_CMP64(...)		((struct scmp_arg_cmp){__VA_ARGS__})
+#define SCMP_CMP		SCMP_CMP64
+
+/**
+ * Specify a 32-bit argument comparison struct for use in declaring rules
+ * @param arg the argument number, starting at 0
+ * @param op the comparison operator, e.g. SCMP_CMP_*
+ * @param datum_a dependent on comparison (32-bits)
+ * @param datum_b dependent on comparison, optional (32-bits)
+ */
+#define SCMP_CMP32(x, y, ...) \
+	_SCMP_MACRO_DISPATCHER(_SCMP_CMP32_, __VA_ARGS__)(x, y, __VA_ARGS__)
+
+/**
+ * Specify a 64-bit argument comparison struct for argument 0
+ */
+#define SCMP_A0_64(...)		SCMP_CMP64(0, __VA_ARGS__)
+#define SCMP_A0			SCMP_A0_64
+
+/**
+ * Specify a 32-bit argument comparison struct for argument 0
+ */
+#define SCMP_A0_32(x, ...)	SCMP_CMP32(0, x, __VA_ARGS__)
+
+/**
+ * Specify a 64-bit argument comparison struct for argument 1
+ */
+#define SCMP_A1_64(...)		SCMP_CMP64(1, __VA_ARGS__)
+#define SCMP_A1			SCMP_A1_64
+
+/**
+ * Specify a 32-bit argument comparison struct for argument 1
+ */
+#define SCMP_A1_32(x, ...)	SCMP_CMP32(1, x, __VA_ARGS__)
+
+/**
+ * Specify a 64-bit argument comparison struct for argument 2
+ */
+#define SCMP_A2_64(...)		SCMP_CMP64(2, __VA_ARGS__)
+#define SCMP_A2			SCMP_A2_64
+
+/**
+ * Specify a 32-bit argument comparison struct for argument 2
+ */
+#define SCMP_A2_32(x, ...)	SCMP_CMP32(2, x, __VA_ARGS__)
 
 /**
- * Specify an argument comparison struct for argument 0
+ * Specify a 64-bit argument comparison struct for argument 3
  */
-#define SCMP_A0(...)		SCMP_CMP(0, __VA_ARGS__)
+#define SCMP_A3_64(...)		SCMP_CMP64(3, __VA_ARGS__)
+#define SCMP_A3			SCMP_A3_64
 
 /**
- * Specify an argument comparison struct for argument 1
+ * Specify a 32-bit argument comparison struct for argument 3
  */
-#define SCMP_A1(...)		SCMP_CMP(1, __VA_ARGS__)
+#define SCMP_A3_32(x, ...)	SCMP_CMP32(3, x, __VA_ARGS__)
 
 /**
- * Specify an argument comparison struct for argument 2
+ * Specify a 64-bit argument comparison struct for argument 4
  */
-#define SCMP_A2(...)		SCMP_CMP(2, __VA_ARGS__)
+#define SCMP_A4_64(...)		SCMP_CMP64(4, __VA_ARGS__)
+#define SCMP_A4			SCMP_A4_64
 
 /**
- * Specify an argument comparison struct for argument 3
+ * Specify a 32-bit argument comparison struct for argument 4
  */
-#define SCMP_A3(...)		SCMP_CMP(3, __VA_ARGS__)
+#define SCMP_A4_32(x, ...)	SCMP_CMP32(4, x, __VA_ARGS__)
 
 /**
- * Specify an argument comparison struct for argument 4
+ * Specify a 64-bit argument comparison struct for argument 5
  */
-#define SCMP_A4(...)		SCMP_CMP(4, __VA_ARGS__)
+#define SCMP_A5_64(...)		SCMP_CMP64(5, __VA_ARGS__)
+#define SCMP_A5			SCMP_A5_64
 
 /**
- * Specify an argument comparison struct for argument 5
+ * Specify a 32-bit argument comparison struct for argument 5
  */
-#define SCMP_A5(...)		SCMP_CMP(5, __VA_ARGS__)
+#define SCMP_A5_32(x, ...)	SCMP_CMP32(5, x, __VA_ARGS__)
 
 /*
  * seccomp actions
diff --git a/tests/48-sim-32b_args.c b/tests/48-sim-32b_args.c
new file mode 100644
index 0000000..2d10519
--- /dev/null
+++ b/tests/48-sim-32b_args.c
@@ -0,0 +1,84 @@
+/**
+ * Seccomp Library test program
+ *
+ * Copyright (c) 2019 Cisco Systems, Inc. <pmoore2@cisco.com>
+ * Author: Paul Moore <paul@paul-moore.com>
+ * Additions: Michael Weiser <michael.weiser@gmx.de>
+ */
+
+/*
+ * 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 <inttypes.h>
+
+#include <seccomp.h>
+
+#include "util.h"
+
+int main(int argc, char *argv[])
+{
+	int rc;
+	struct util_options opts;
+	scmp_filter_ctx ctx = NULL;
+	struct args {
+		uint32_t action;
+		int syscall;
+		struct scmp_arg_cmp cmp;
+	} *a, f[] = {
+		{SCMP_ACT_ALLOW, 2000, SCMP_A0(SCMP_CMP_EQ, -1)},
+		{SCMP_ACT_ALLOW, 2064, SCMP_A0_64(SCMP_CMP_EQ, -1)},
+		{SCMP_ACT_ALLOW, 2032, SCMP_A0_32(SCMP_CMP_EQ, -1)},
+		{0},
+	};
+
+	rc = util_getopt(argc, argv, &opts);
+	if (rc < 0)
+		goto out;
+
+	ctx = seccomp_init(SCMP_ACT_KILL);
+	if (ctx == NULL)
+		return ENOMEM;
+
+	rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1000, 1,
+				    SCMP_A0(SCMP_CMP_EQ, -1));
+	if (rc != 0)
+		goto out;
+
+	rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1064, 1,
+				    SCMP_A0_64(SCMP_CMP_EQ, -1));
+	if (rc != 0)
+		goto out;
+
+	rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1032, 1,
+				    SCMP_A0_32(SCMP_CMP_EQ, -1));
+	if (rc != 0)
+		goto out;
+
+	for (a = f; a->syscall != 0; a++) {
+		rc = seccomp_rule_add_exact(ctx, a->action, a->syscall, 1,
+					    a->cmp);
+		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/48-sim-32b_args.py b/tests/48-sim-32b_args.py
new file mode 100755
index 0000000..486c488
--- /dev/null
+++ b/tests/48-sim-32b_args.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+
+#
+# Seccomp Library test program
+#
+# Copyright (c) 2019 Cisco Systems, Inc. <pmoore2@cisco.com>
+# Author: Paul Moore <paul@paul-moore.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(KILL)
+    # NOTE: this test is different from the native/c test as the bindings don't
+    #       allow negative numbers (which is a good thing here)
+    f.add_rule_exactly(ALLOW, 1000, Arg(0, EQ, 0xffffffffffffffff))
+    f.add_rule_exactly(ALLOW, 1064, Arg(0, EQ, 0xffffffffffffffff))
+    f.add_rule_exactly(ALLOW, 1032, Arg(0, EQ, 0xffffffff))
+    # here we do not have static initializers to test but need to keep
+    # behaviour in sync with the native test
+    f.add_rule_exactly(ALLOW, 2000, Arg(0, EQ, 0xffffffffffffffff))
+    f.add_rule_exactly(ALLOW, 2064, Arg(0, EQ, 0xffffffffffffffff))
+    f.add_rule_exactly(ALLOW, 2032, Arg(0, EQ, 0xffffffff))
+    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/48-sim-32b_args.tests b/tests/48-sim-32b_args.tests
new file mode 100644
index 0000000..b19fe5e
--- /dev/null
+++ b/tests/48-sim-32b_args.tests
@@ -0,0 +1,38 @@
+#
+# libseccomp regression test automation data
+#
+# Copyright (c) 2019 Cisco Systems, Inc. <pmoore2@cisco.com>
+# Author: Paul Moore <paul@paul-moore.com>
+#
+
+test type: bpf-sim
+
+# Testname		Arch	Syscall	Arg0			Arg1	Arg2	Arg3	Arg4	Arg5	Result
+48-sim-32b_args		all	1000	0x0			N	N	N	N	N	KILL
+48-sim-32b_args		all	1000	0xffffffff		N	N	N	N	N	KILL
+48-sim-32b_args		all	1000	0xffffffffffffffff	N	N	N	N	N	ALLOW
+48-sim-32b_args		all	1032	0x0			N	N	N	N	N	KILL
+48-sim-32b_args		all	1032	0xffffffff		N	N	N	N	N	ALLOW
+48-sim-32b_args		all	1032	0xffffffffffffffff	N	N	N	N	N	KILL
+48-sim-32b_args		all	1064	0x0			N	N	N	N	N	KILL
+48-sim-32b_args		all	1064	0xffffffff		N	N	N	N	N	KILL
+48-sim-32b_args		all	1064	0xffffffffffffffff	N	N	N	N	N	ALLOW
+48-sim-32b_args		all	2000	0x0			N	N	N	N	N	KILL
+48-sim-32b_args		all	2000	0xffffffff		N	N	N	N	N	KILL
+48-sim-32b_args		all	2000	0xffffffffffffffff	N	N	N	N	N	ALLOW
+48-sim-32b_args		all	2032	0x0			N	N	N	N	N	KILL
+48-sim-32b_args		all	2032	0xffffffff		N	N	N	N	N	ALLOW
+48-sim-32b_args		all	2032	0xffffffffffffffff	N	N	N	N	N	KILL
+48-sim-32b_args		all	2064	0x0			N	N	N	N	N	KILL
+48-sim-32b_args		all	2064	0xffffffff		N	N	N	N	N	KILL
+48-sim-32b_args		all	2064	0xffffffffffffffff	N	N	N	N	N	ALLOW
+
+test type: bpf-sim-fuzz
+
+# Testname		StressCount
+48-sim-32b_args		50
+
+test type: bpf-valgrind
+
+# Testname
+48-sim-32b_args
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 07e1654..365ae7e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -86,7 +86,8 @@ check_PROGRAMS = \
 	44-live-a2_order \
 	45-sim-chain_code_coverage \
 	46-sim-kill_process \
-	47-live-kill_process
+	47-live-kill_process \
+	48-sim-32b_args
 
 EXTRA_DIST_TESTPYTHON = \
 	util.py \
@@ -135,7 +136,8 @@ EXTRA_DIST_TESTPYTHON = \
 	44-live-a2_order.py \
 	45-sim-chain_code_coverage.py \
 	46-sim-kill_process.py \
-	47-live-kill_process.py
+	47-live-kill_process.py \
+	48-sim-32b_args.py
 
 EXTRA_DIST_TESTCFGS = \
 	01-sim-allow.tests \
@@ -184,7 +186,8 @@ EXTRA_DIST_TESTCFGS = \
 	44-live-a2_order.tests \
 	45-sim-chain_code_coverage.tests \
 	46-sim-kill_process.tests \
-	47-live-kill_process.tests
+	47-live-kill_process.tests \
+	48-sim-32b_args.tests
 
 EXTRA_DIST_TESTSCRIPTS = \
 	38-basic-pfc_coverage.sh 38-basic-pfc_coverage.pfc