Discussion:
[libseccomp-discuss] [PATCH 1/2] AArch64 support v7
Marcin Juszkiewicz
2014-08-21 15:30:40 UTC
Permalink
Changelog

v7 - 2014.08.21

- rebased on top of git
- created new syscall table based on 3.17-rc1 kernel
- added more legacy syscalls as __PNR macros
- merged patches from Akashi Takahiro
- changed comment about AArch64 audit constant
- changes tests to use openat() instead of legacy open()

Tested on Mustang running 3.17-rc1 Fedora with 64KB pages and Akashi Takahiro seccomp v5 patches.

Regression Test Summary
tests run: 8150
tests skipped: 138
tests passed: 8139
tests failed: 11
tests errored: 5


V6 - 2014.07.30

- rebased on top of git
- added AArch64 handling to src/arch.c and tools/util.c

Tested on Mustang running 3.16-rc7 Fedora with 64KB pages and
Akashi Takahiro (Cc:) audit v10 + seccomp v5 patches.

Regression Test Summary
tests run: 459
tests skipped: 79
tests passed: 453
tests failed: 6
tests errored: 164

I had OOM running for each test after 02-sim-basic (on 16GB ram + 8GB swap).


V4 - 2014.06.04

- rebased on top of git

Regression Test Summary
tests run: 935
tests skipped: 59
tests passed: 902
tests failed: 33
tests errored: 141


V3 - 04.06.2014

- added AArch64 detection into tools/
- added AArch64 detection into tests/{16,23}

Regression Test Summary
tests run: 423
tests skipped: 59
tests passed: 403
tests failed: 20
tests errored: 167


V2 - 03.06.2014

- filled syscalls table
- added new __PNR_ macros for alarm, mmap, open, select, time, utime


V1 - 28.05.2014
Marcin Juszkiewicz
2014-08-21 15:36:48 UTC
Permalink
Before:

Regression Test Summary
tests run: 8008
tests skipped: 138
tests passed: 7997
tests failed: 11
tests errored: 17

http://malenstwo.juszkiewicz.com.pl/~hrw/test-0821-1551-open.log

After:

Regression Test Summary
tests run: 8150
tests skipped: 138
tests passed: 8139
tests failed: 11
tests errored: 5

http://malenstwo.juszkiewicz.com.pl/~hrw/test-0821-1224-openat.log
Paul Moore
2014-08-21 19:55:46 UTC
Permalink
Post by Marcin Juszkiewicz
Regression Test Summary
tests run: 8008
tests skipped: 138
tests passed: 7997
tests failed: 11
tests errored: 17
http://malenstwo.juszkiewicz.com.pl/~hrw/test-0821-1551-open.log
Regression Test Summary
tests run: 8150
tests skipped: 138
tests passed: 8139
tests failed: 11
tests errored: 5
http://malenstwo.juszkiewicz.com.pl/~hrw/test-0821-1224-openat.log
Marcin Juszkiewicz
2014-08-20 20:07:12 UTC
Permalink
Some architectures (like AArch64) do not support legacy syscalls. Using
them generates failures in test suite which disapear when openat() is
used instead.

Signed-off-by: Marcin Juszkiewicz <***@redhat.com>
---
tests/04-sim-multilevel_chains.c | 2 +-
tests/04-sim-multilevel_chains.py | 2 +-
tests/04-sim-multilevel_chains.tests | 2 +-
tests/06-sim-actions.py | 2 +-
tests/15-basic-resolver.py | 6 +++---
tests/21-live-basic_allow.py | 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/04-sim-multilevel_chains.c
b/tests/04-sim-multilevel_chains.c
index 83bbfd5..bbeb1f1 100644
--- a/tests/04-sim-multilevel_chains.c
+++ b/tests/04-sim-multilevel_chains.c
@@ -41,7 +41,7 @@ int main(int argc, char *argv[])
if (ctx == NULL)
return ENOMEM;

- rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), 0);
+ rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), 0);
if (rc != 0)
goto out;

diff --git a/tests/04-sim-multilevel_chains.py
b/tests/04-sim-multilevel_chains.py
index e40deee..a1e6e92 100755
--- a/tests/04-sim-multilevel_chains.py
+++ b/tests/04-sim-multilevel_chains.py
@@ -30,7 +30,7 @@ from seccomp import *

def test(args):
f = SyscallFilter(KILL)
- f.add_rule_exactly(ALLOW, "open");
+ f.add_rule_exactly(ALLOW, "openat");
f.add_rule_exactly(ALLOW, "close");
f.add_rule_exactly(ALLOW, "read",
Arg(0, EQ, sys.stdin.fileno()),
diff --git a/tests/04-sim-multilevel_chains.tests
b/tests/04-sim-multilevel_chains.tests
index cefbc4f..1ea50f4 100644
--- a/tests/04-sim-multilevel_chains.tests
+++ b/tests/04-sim-multilevel_chains.tests
@@ -8,7 +8,7 @@
test type: bpf-sim

# Testname Arch Syscall Arg0 Arg1 Arg2 Arg3 Arg4 Arg5 Result
-04-sim-multilevel_chains all open 0x856B008 4 N N N N ALLOW
+04-sim-multilevel_chains all openat 0x856B008 4 N N N N ALLOW
04-sim-multilevel_chains all close 4 N N N N N ALLOW
04-sim-multilevel_chains x86 read 0 0x856B008 0x7FFFFFFE N N N ALLOW
04-sim-multilevel_chains x86_64 read 0 0x856B008
0x7FFFFFFFFFFFFFFE N N N ALLOW
diff --git a/tests/06-sim-actions.py b/tests/06-sim-actions.py
index 4bd76f5..78655b2 100755
--- a/tests/06-sim-actions.py
+++ b/tests/06-sim-actions.py
@@ -34,7 +34,7 @@ def test(args):
f.add_rule(ALLOW, "read")
f.add_rule(ERRNO(errno.EPERM), "write")
f.add_rule(TRAP, "close")
- f.add_rule(TRACE(1234), "open")
+ f.add_rule(TRACE(1234), "openat")
return f

args = util.get_opt()
diff --git a/tests/15-basic-resolver.py b/tests/15-basic-resolver.py
index 6009d6d..e3fe419 100755
--- a/tests/15-basic-resolver.py
+++ b/tests/15-basic-resolver.py
@@ -32,16 +32,16 @@ def test():
f = SyscallFilter(KILL)
# this differs from the native test as we don't support the syscall
# resolution functions by themselves
- f.add_rule(ALLOW, "open")
+ f.add_rule(ALLOW, "openat")
f.add_rule(ALLOW, "socket")
try:
f.add_rule(ALLOW, "INVALID")
except RuntimeError:
pass

- sys_num = resolve_syscall(Arch(), "open")
+ sys_num = resolve_syscall(Arch(), "openat")
sys_name = resolve_syscall(Arch(), sys_num)
- if (sys_name != "open"):
+ if (sys_name != "openat"):
raise RuntimeError("Test failure")
sys_num = resolve_syscall(Arch(), "socket")
sys_name = resolve_syscall(Arch(), sys_num)
diff --git a/tests/21-live-basic_allow.py b/tests/21-live-basic_allow.py
index 1332f2e..1fa7907 100755
--- a/tests/21-live-basic_allow.py
+++ b/tests/21-live-basic_allow.py
@@ -37,7 +37,7 @@ def test():
# NOTE: additional syscalls required for python
f.add_rule_exactly(ALLOW, "stat")
f.add_rule_exactly(ALLOW, "fstat")
- f.add_rule_exactly(ALLOW, "open")
+ f.add_rule_exactly(ALLOW, "openat")
f.add_rule_exactly(ALLOW, "mmap")
f.add_rule_exactly(ALLOW, "munmap")
f.add_rule_exactly(ALLOW, "read")
--
2.1.0
AKASHI Takahiro
2014-08-22 01:53:32 UTC
Permalink
Marcin,
Post by Marcin Juszkiewicz
Changelog
v7 - 2014.08.21
I think that you'd better split your patch [1/2] into smaller chunks of patches for better reviewing.
In addition, it might be preferable to put aarch64-related code just after arm-related code.
The current layout/sequence like
...
arm
mips64
mips64n32
aarch64
...
looks to be disordered. We also have some possibilities to add big endian support, if necessary,
for arm/arm64 in the future.
Post by Marcin Juszkiewicz
- rebased on top of git
- created new syscall table based on 3.17-rc1 kernel
- added more legacy syscalls as __PNR macros
I'm not sure, but do we really need those annoying definitions?
Post by Marcin Juszkiewicz
- merged patches from Akashi Takahiro
- changed comment about AArch64 audit constant
- changes tests to use openat() instead of legacy open()
Obviously, thinking of other archs, you should not replace open() with openat() unconditionally.
(This is one of reasons that I hesitated to make my patches published.)
Post by Marcin Juszkiewicz
Tested on Mustang running 3.17-rc1 Fedora with 64KB pages and Akashi Takahiro seccomp v5 patches.
Thanks, try also my new v6, (but which will be soon followed by v7 :)
Post by Marcin Juszkiewicz
Regression Test Summary
tests run: 8150
tests skipped: 138
tests passed: 8139
tests failed: 11
tests errored: 5
V6 - 2014.07.30
- rebased on top of git
- added AArch64 handling to src/arch.c and tools/util.c
Tested on Mustang running 3.16-rc7 Fedora with 64KB pages and
Akashi Takahiro (Cc:) audit v10 + seccomp v5 patches.
Regression Test Summary
tests run: 459
tests skipped: 79
tests passed: 453
tests failed: 6
tests errored: 164
I had OOM running for each test after 02-sim-basic (on 16GB ram + 8GB swap).
V4 - 2014.06.04
- rebased on top of git
Regression Test Summary
tests run: 935
tests skipped: 59
tests passed: 902
tests failed: 33
tests errored: 141
V3 - 04.06.2014
- added AArch64 detection into tools/
- added AArch64 detection into tests/{16,23}
Regression Test Summary
tests run: 423
tests skipped: 59
tests passed: 403
tests failed: 20
tests errored: 167
V2 - 03.06.2014
- filled syscalls table
- added new __PNR_ macros for alarm, mmap, open, select, time, utime
V1 - 28.05.2014
Paul Moore
2014-08-22 05:05:59 UTC
Permalink
Post by AKASHI Takahiro
Marcin,
Post by Marcin Juszkiewicz
Changelog
v7 - 2014.08.21
I think that you'd better split your patch [1/2] into smaller chunks of
patches for better reviewing.
Yes, it could be split up a bit more (see what I did for 64-bit MIPS), but it
is okay. Adding a new ABI is always going to be a huge change and it is
sometimes hard to break it up into a series of smaller atomic changes that
still accomplish anything meaningful.
Post by AKASHI Takahiro
In addition, it might be preferable to put
aarch64-related code just after arm-related code. The current
layout/sequence like
...
arm
mips64
mips64n32
aarch64
...
looks to be disordered. We also have some possibilities to add big endian
support, if necessary, for arm/arm64 in the future.
True, this is probably the right thing to do, but I'm not too hung up on it at
the moment as it is a trivial thing to change in the future.
Post by AKASHI Takahiro
Post by Marcin Juszkiewicz
- rebased on top of git
- created new syscall table based on 3.17-rc1 kernel
- added more legacy syscalls as __PNR macros
I'm not sure, but do we really need those annoying definitions?
I've been spending a good chunk of time reviewing/fixing/reworking Marcin's
patch today/tonight and while most of the patch is pretty close, the syscall
table has some significant problems. I'm going to be posting an updated patch
shortly that you and Marcin can test/review.
--
paul moore
security and virtualization @ redhat
Loading...