Compressed syscall database with O(1) lookup #21
Conversation
mejedi
force-pushed
the
master
branch
6 times, most recently
from
c466073
to
2f6b76f
Compare
mejedi
changed the title
libkafel.so 5x smaller (x86_64, stripped): down to 88KiB from 440KiB. Closes google#20
|
@happyCoder92 Could you please take a look? |
There was a problem hiding this comment.
Sorry for the long delay.
The binary size reduction is quite impressive, but I'm not sure if it's worth the added complexity, as in case of constrained system one can just compile the syscall tables for one architecture (which is a lot simpler and brings the library size ~120KB).
Given the complexity of the change an exhaustive test against the regular tables would be nice.
The syscalldb_generator does not compile cleanly with clang.
is_custom from syscall_descriptor and NORMAL from the tables should be removed as it is no longer needed.
| gperf -m10 --output-file=./syscalldb.c ./syscalldb.gperf | ||
|
|
||
| syscalls/syscalldb_generator: syscalls | ||
| true |
| @@ -70,7 +70,6 @@ void kafel_ctxt_reset(kafel_ctxt_t ctxt) { | |||
| } | |||
| ctxt->default_action = 0; | |||
| ctxt->lexical_error = false; | |||
| ctxt->syscalls = NULL; | |||
There was a problem hiding this comment.
should clear target_arch_mask
| @@ -298,7 +298,7 @@ syscall_id | |||
| $$ = syscall_custom(value); | |||
| } else { | |||
| $$ = (struct syscall_descriptor*) | |||
There was a problem hiding this comment.
cast not needed anymore
| @@ -46,7 +46,7 @@ struct kafel_ctxt { | |||
| struct policy* main_policy; | |||
| int default_action; | |||
| uint32_t target_arch; | |||
| const struct syscall_list* syscalls; | |||
| uint32_t target_arch_mask; | |||
There was a problem hiding this comment.
use just int instead of uint32_t
name is a little bit misleading, maybe use syscalldb_arch
| } | ||
|
|
||
| const struct syscall_descriptor* syscall_lookup(const struct syscall_list* list, | ||
| const struct syscall_descriptor* syscall_lookup(uint32_t mask, |
There was a problem hiding this comment.
remove const from return type
| }; | ||
|
|
||
| const struct syscalldb_definition* syscalldb_lookup(const char* name); | ||
| const char* syscalldb_reverse_lookup(uint32_t arch_mask, uint32_t nr); |
There was a problem hiding this comment.
unused, please remove
| uint32_t arch_mask, struct syscall_descriptor* dest); | ||
|
|
||
| /* | ||
| internals |
There was a problem hiding this comment.
move internals out of the header
| @@ -0,0 +1,84 @@ | |||
| /* | |||
There was a problem hiding this comment.
name should be syscalldb.inc
preferably code could be refactored to make it a normal .c file.
| const char* syscalldb_reverse_lookup(uint32_t arch_mask, uint32_t nr); | ||
|
|
||
| void syscalldb_unpack(const struct syscalldb_definition* definition, | ||
| uint32_t arch_mask, struct syscall_descriptor* dest); |
There was a problem hiding this comment.
it does not make sense to call it with a mask like SYSCALLDB_ARCH_ARM_FLAG|SYSCALLDB_ARCH_AARCH64_FLAG and it won't work in current implementation.
Maybe just use masks internally and externally just consecutive values for arch enum or even AUDIT_ARCH_*.
|
@happyCoder92 I'd like to discuss the overall direction before moving forward. I am using Kafel in https://github.com/rapidlua/sandals, which is a lightweight sandbox similar to nsjail. Sandals are used in https://luajit.me for secure execution of user-submitted Lua code. Sandbox overhead as opposed to running a process unsandboxed is a mere 5ms. I need access to syscall database so that the sandbox could produce a description of the syscall denied. I assume that other users might have similar needs. My suggestion is to split the database into a separate library, say |
|
Spliting the syscall database into separate library sounds like a good idea. |
libkafel.so 5x smaller (x86_64, stripped): down to 88KiB from 440KiB.
Closes #20