bpf: fix x32/x86_64 architecture detection logic

Test 28-sim-arch_x86 points out a flaw in the x32 arch handling as we
wrongly jump to the next architecture check while we should jump to the
bad_arch handling instruction instead. See below:

$ ./tests/28-sim-arch_x86 -b | ./tools/scmp_bpf_disasm
 line  OP   JT   JF   K
=================================
 0000: 0x20 0x00 0x00 0x00000004   ld  $data[4]
 0001: 0x15 0x00 0x03 0xc000003e   jeq 3221225534 true:0002 false:0005
 0002: 0x20 0x00 0x00 0x00000000   ld  $data[0]
 0003: 0x35 0x01 0x00 0x40000000   jge 1073741824 true:0005 false:0004
 0004: 0x15 0x04 0x03 0x00000003   jeq 3    true:0009 false:0008
 0005: 0x15 0x00 0x04 0x40000003   jeq 1073741827 true:0006 false:0010
 0006: 0x20 0x00 0x00 0x00000000   ld  $data[0]
 0007: 0x15 0x01 0x00 0x00000006   jeq 6    true:0009 false:0008
 0008: 0x06 0x00 0x00 0x7fff0000   ret ALLOW
 0009: 0x06 0x00 0x00 0x00050001   ret ERRNO(1)
 0010: 0x06 0x00 0x00 0x00000000   ret KILL

When we reach the test at 0003 the accumulator register was changed
from holding the audit architecture to contain the syscall number
instead. This is needed to actually test for the x32 sub-architecture
as it, unfortunately, got no dedicated audit arch value. However, if
that test succeeds, we end up jumping to the next architecture check
at 0005 which is wrong. We should jump to the bad_arch handling at
0010 instead as x32 is an unsupported architecture for that test
program. Even worse, the next architecture check now operates on the
wrong data as it's no longer testing the audit arch but the syscall
number instead. As it happen to be, the syscall number for x32's
close() is 0x40000003. That exactly matches the audit arch value for
the x86 architecture. So what this filter does is allowing the x32
close() call while it should not.

As we already successfully checked the arch to be SCMP_ARCH_X86_64 in
0001 it cannot have a different value. Testing for other values just
makes no sense. So instead of reloading the accumulator register on a
successful x32 test fix this by jumping to the bad_arch handling block
instead.

The generated BPF program now looks as follows:

$ ./tests/28-sim-arch_x86 -b | ./tools/scmp_bpf_disasm
 line  OP   JT   JF   K
=================================
 0000: 0x20 0x00 0x00 0x00000004   ld  $data[4]
 0001: 0x15 0x00 0x03 0xc000003e   jeq 3221225534 true:0002 false:0005
 0002: 0x20 0x00 0x00 0x00000000   ld  $data[0]
 0003: 0x35 0x06 0x00 0x40000000   jge 1073741824 true:0010 false:0004
 0004: 0x15 0x04 0x03 0x00000003   jeq 3    true:0009 false:0008
 0005: 0x15 0x00 0x04 0x40000003   jeq 1073741827 true:0006 false:0010
 0006: 0x20 0x00 0x00 0x00000000   ld  $data[0]
 0007: 0x15 0x01 0x00 0x00000006   jeq 6    true:0009 false:0008
 0008: 0x06 0x00 0x00 0x7fff0000   ret ALLOW
 0009: 0x06 0x00 0x00 0x00050001   ret ERRNO(1)
 0010: 0x06 0x00 0x00 0x00000000   ret KILL

It now correctly jumps to the bad_arch handling at 0010 when the x32
test in 0003 succeeds.

This fixes test 28-sim-arch_x86.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
[PM: subject tweak, renamed 'bad_arch_hash' to 'bad_arch_hsh']
Signed-off-by: Paul Moore <pmoore@redhat.com>
(imported from commit 8868f7eb0c343cfcb9bbe28736928a7f7e108b97)

3 files changed, 11 insertions, 4 deletions

diff --git a/src/gen_bpf.c b/src/gen_bpf.c
index 452d954..8d1b1b5 100644
--- a/src/gen_bpf.c
+++ b/src/gen_bpf.c
@@ -153,6 +153,8 @@ struct bpf_state {
 
 	/* filter attributes */
 	const struct db_filter_attr *attr;
+	/* bad arch action */
+	uint64_t bad_arch_hsh;
 	/* default action */
 	uint64_t def_hsh;
 
@@ -1352,22 +1354,26 @@ static struct bpf_blk *_gen_bpf_arch(struct bpf_state *state,
 			/* filter out x32 */
 			_BPF_INSTR(instr,
 				   _BPF_OP(state->arch, BPF_JMP + BPF_JGE),
-				   _BPF_JMP_NXT(blk_cnt++), _BPF_JMP_NO,
+				   _BPF_JMP_HSH(state->bad_arch_hsh),
+				   _BPF_JMP_NO,
 				   _BPF_K(state->arch, X32_SYSCALL_BIT));
 			if (b_head != NULL)
 				instr.jf = _BPF_JMP_HSH(b_head->hash);
 			else
 				instr.jf = _BPF_JMP_HSH(state->def_hsh);
+			blk_cnt++;
 		} else if (state->arch->token == SCMP_ARCH_X32) {
 			/* filter out x86_64 */
 			_BPF_INSTR(instr,
 				   _BPF_OP(state->arch, BPF_JMP + BPF_JGE),
-				   _BPF_JMP_NO, _BPF_JMP_NXT(blk_cnt++),
+				   _BPF_JMP_NO,
+				   _BPF_JMP_HSH(state->bad_arch_hsh),
 				   _BPF_K(state->arch, X32_SYSCALL_BIT));
 			if (b_head != NULL)
 				instr.jt = _BPF_JMP_HSH(b_head->hash);
 			else
 				instr.jt = _BPF_JMP_HSH(state->def_hsh);
+			blk_cnt++;
 		} else
 			/* we should never get here */
 			goto arch_failure;
@@ -1636,6 +1642,7 @@ static int _gen_bpf_build_bpf(struct bpf_state *state,
 	rc = _hsh_add(state, &b_badarch, 1);
 	if (rc < 0)
 		return rc;
+	state->bad_arch_hsh = b_badarch->hash;
 
 	/* generate the default action */
 	b_default = _gen_bpf_action(state, NULL, state->attr->act_default);
diff --git a/tests/28-sim-arch_x86.c b/tests/28-sim-arch_x86.c
index e93c0a7..fa6302f 100644
--- a/tests/28-sim-arch_x86.c
+++ b/tests/28-sim-arch_x86.c
@@ -1,7 +1,7 @@
 /**
  * Seccomp Library test program
  *
- * This test triggers a bug in libseccomp erroneously allowing the close()
+ * This test triggered a bug in libseccomp erroneously allowing the close()
  * syscall on x32 instead of 'KILL'ing it, as it should do for unsupported
  * architectures.
  *
diff --git a/tests/28-sim-arch_x86.tests b/tests/28-sim-arch_x86.tests
index b86a047..45978aa 100644
--- a/tests/28-sim-arch_x86.tests
+++ b/tests/28-sim-arch_x86.tests
@@ -1,7 +1,7 @@
 #
 # libseccomp regression test automation data
 #
-# This test triggers a bug in libseccomp erroneously allowing the close()
+# This test triggered a bug in libseccomp erroneously allowing the close()
 # syscall on x32 instead of 'KILL'ing it, as it should do for unsupported
 # architectures.
 #