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 <[email protected]>
Signed-off-by: Michael Weiser <[email protected]>

doc/man/man3/seccomp_rule_add.3

@@ -1,4 +1,4 @@
-.TH "seccomp_rule_add" 3 "25 July 2012" "[email protected]" "libseccomp Documentation"
+.TH "seccomp_rule_add" 3 "17 February 2019" "[email protected]" "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

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

tests/48-sim-32b_args.c

@@ -0,0 +1,84 @@
+/**
+ * Seccomp Library test program
+ *
+ * Copyright (c) 2019 Cisco Systems, Inc. <[email protected]>
+ * Author: Paul Moore <[email protected]>
+ * Additions: Michael Weiser <[email protected]>
+ */
+
+/*
+ * 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:https://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);
+}

tests/48-sim-32b_args.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+
+#
+# Seccomp Library test program
+#
+# Copyright (c) 2019 Cisco Systems, Inc. <[email protected]>
+# Author: Paul Moore <[email protected]>
+#
+
+#
+# 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:https://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;

tests/48-sim-32b_args.tests

@@ -0,0 +1,38 @@
+#
+# libseccomp regression test automation data
+#
+# Copyright (c) 2019 Cisco Systems, Inc. <[email protected]>
+# Author: Paul Moore <[email protected]>
+#
+
+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

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