db: reject conflicting seccomp rule actions

doc/man/man3/seccomp_rule_add.3

@@ -99,6 +99,13 @@ argument values and later a rule is added which allows the same syscall
 regardless the argument values then the first, more specific rule, is
 effectively dropped from the filter by the second more generic rule.
 .P
+When an unconditional rule conflicts with an existing rule for the same
+system call, because their actions differ, the new rule is rejected with
+.B -EEXIST
+instead of silently discarding either rule.  This applies whether the
+unconditional rule is added before or after the conditional rule.  A second
+unconditional rule with a different action is also rejected.
+.P
 The
 .BR SCMP_CMP (),
 .BR SCMP_CMP64 (),
@@ -336,7 +343,7 @@ failure:
 Architecture specific failure.
 .TP
 .B -EEXIST
-The rule already exists.
+The rule conflicts with an existing rule.
 .TP
 .B -EACCES
 The rule conflicts with the filter (for example, the rule

src/db.c

@@ -2144,13 +2144,22 @@ int db_rule_add(struct db_filter *db, const struct db_api_rule_list *rule)
 			/* syscall exists without any chains - existing filter
 			 * is at least as large as the new entry so cleanup and
 			 * exit */
+			/* Do not silently discard a rule with a different action. */
+			if (s_iter->action != rule->action) {
+				rc = -EEXIST;
+				goto add_failure;
+			}
 			_db_tree_put(&s_new->chains);
 			free(s_new);
 			goto add_free_ok;
 		}
 	} else if (s_iter->chains != NULL && s_new->chains == NULL) {
 		/* syscall exists with chains but the new filter has no chains
 		 * so we need to clear the existing chains and exit */
+		/* Keep the existing conditional rules if an action conflicts. */
+		rc = _db_tree_act_check(s_iter->chains, rule->action);
+		if (rc < 0)
+			goto add_failure;
 		_db_tree_put(&s_iter->chains);
 		s_iter->chains = NULL;
 		s_iter->node_cnt = 0;

tests/11-basic-basic_errors.c

@@ -117,6 +117,40 @@ int main(int argc, char *argv[])
 	seccomp_release(ctx);
 	ctx = NULL;
 
+	/* seccomp_rule_add conflicting actions */
+	ctx = seccomp_init(SCMP_ACT_ERRNO(EPERM));
+	if (ctx == NULL)
+		return -1;
+	rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(getpid), 1,
+			      SCMP_A0(SCMP_CMP_EQ, 1));
+	if (rc != 0)
+		return -1;
+	rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(getpid), 0);
+	if (rc != -EEXIST)
+		return -1;
+	/* A same-action unconditional rule can still subsume the conditional. */
+	rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(getpid), 0);
+	if (rc != 0)
+		return -1;
+	seccomp_release(ctx);
+	ctx = NULL;
+
+	ctx = seccomp_init(SCMP_ACT_ERRNO(EPERM));
+	if (ctx == NULL)
+		return -1;
+	rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(getpid), 0);
+	if (rc != 0)
+		return -1;
+	rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(getpid), 1,
+			      SCMP_A0(SCMP_CMP_EQ, 1));
+	if (rc != -EEXIST)
+		return -1;
+	rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(getpid), 0);
+	if (rc != -EEXIST)
+		return -1;
+	seccomp_release(ctx);
+	ctx = NULL;
+
 	/* seccomp_rule_add_exact error */
 	ctx = seccomp_init(SCMP_ACT_ALLOW);
 	if (ctx == NULL)

tests/11-basic-basic_errors.py

@@ -22,6 +22,7 @@
 #
 
 import argparse
+import errno
 import sys
 
 import util
@@ -73,6 +74,33 @@ def test():
     except RuntimeError:
         pass
 
+    # Conflicting unconditional and conditional rules must not be silently
+    # discarded.
+    f = SyscallFilter(ERRNO(errno.EPERM))
+    f.add_rule(KILL, "getpid", Arg(0, EQ, 1))
+    try:
+        f.add_rule(ALLOW, "getpid")
+    except RuntimeError:
+        pass
+    else:
+        raise RuntimeError("expected conflicting rule to fail")
+    f.add_rule(KILL, "getpid")
+
+    f = SyscallFilter(ERRNO(errno.EPERM))
+    f.add_rule(ALLOW, "getpid")
+    try:
+        f.add_rule(KILL, "getpid", Arg(0, EQ, 1))
+    except RuntimeError:
+        pass
+    else:
+        raise RuntimeError("expected conflicting rule to fail")
+    try:
+        f.add_rule(KILL, "getpid")
+    except RuntimeError:
+        pass
+    else:
+        raise RuntimeError("expected conflicting rule to fail")
+
     f = SyscallFilter(ALLOW)
     f.remove_arch(Arch())
     f.add_arch(Arch("x86"))