Syscalls py (#3106)

[rusty-snake]

@topimiettinen

[lgtm-com]

This pull request introduces 1 alert when merging 45701dd into 0c73dbc - view on LGTM.com

new alerts:

• 1 for Unused import

[rusty-snake]

forgotten to remove.

[reinerh]

I like that the file can now be automatically be generated. 👍
But if possible, this should be done during build and the generated file should not be in git, because the syscalls in the list might not be the ones supported on the system where it is built. (Or can firejail deal with the fact that there are syscalls in the list that are unsupported?)

What I don't like so much, is that it uses systemd-analyze and therefore adds a build-dependency on the systemd package, which makes it more complicated to build on systems/distros not using systemd.

[rusty-snake]

What I don't like so much, is that it uses systemd-analyze and therefore adds a build-dependency on the systemd package, which makes it more complicated to build on systems/distros not using systemd.

That's why I added it this way. However, we could add it to git and rebuild if systemd-analyze is available.

because the syscalls in the list might not be the ones supported on the system where it is built.

By arch? IDK @topimiettinen can say more, but as I get it in #3624 it works.
If you mean by the kernel version, we have one firejail.deb on sourceforge that is used by debain, ubuntu and mint users.

[reinerh]

By arch? IDK @topimiettinen can say more, but as I get it in #3624 it works.
If you mean by the kernel version, we have one firejail.deb on sourceforge that is used by debain, ubuntu and mint users.

By either kernel version or arch. If this list is built on a system with a recent kernel, it will include syscalls that are not supported on older systems (this was protected before with #ifdef, so it was only available when the system being built for supports it).
I'm not sure where firejail.deb from sourceforge is built. If it's an old Debian/Ubuntu that's probably fine, as it contained only syscalls supported on these systems or newer (but might miss others).
But now the list contains all syscalls, so when a package is for example built for a backport, it is built against older kernels not supporting it (and will also run on systems not supporting it).
That's also why I asked if firejail can deal with syscalls in the list, that are not supported.

[reinerh]

Ok, if I understand #3106 correctly, then firejail is ignoring unknown/invalid syscall names. Then please ignore my comment. :-)
(Though it's still not completely clear to me how syscalls of other archs will land in the list)

[topimiettinen]

Firejail's src/include/syscall_x86_64.h etc. do not care if the libc defines the syscalls or not, they are always available. If Firejail is running on an old kernel, it's possible to filter newer system calls than the kernel expects but this is OK. Seccomp BPF code just compares register values to constants and so it doesn't matter if these are system call numbers or something else. Also the applications can always call anything they want with syscall(2), regardless of the libc or kernel support.

systemd-analyze syscall-filter prints system calls for all archs except for two s390 ones, which is nice. But comparing to our ARM system calls, at least arm_fadvise64_64 and arm_sync_file_range are missing. Perhaps libseccomp translates generic names on ARM to ARMish. Maybe syscalls.py should do the same in reverse?

Another approach would be copying relevant parts of systemd src/shared/seccomp-util.c directly, it's LGPL2.1+. It could be also pre-processed to extract the tables and replace NULs with commas. It wouldn't help with the arch problem though.

Yet another approach would be changing over to libseccomp, but it's nice that Firejail does not depend on lots of libraries. Also direct BPF is more powerful.

[rusty-snake]

But comparing to our ARM system calls, at least arm_fadvise64_64 and arm_sync_file_range are missing. Perhaps libseccomp translates generic names on ARM to ARMish.

I'm not sure if I get you right.

Maybe syscalls.py should do the same in reverse?

Sounds good, but I haven't found it in libseccomps repo.
https://github.com/seccomp/libseccomp/search?q=arm_sync_file_range&unscoped_q=arm_sync_file_range

[topimiettinen]

I expected fadvise64_64 and sync_file_range to be translated on ARM to arm_fadvise64_64 and arm_sync_file_range or the other way around, but maybe that's not the case.

Other sources for system calls could be libseccomp src/syscalls.csv, systemd src/shared/syscall-names.text and various kernel files like arch/arm/tools/syscall.tbl.