seccomp / libseccomp

The main libseccomp repository
GNU Lesser General Public License v2.1
794 stars 171 forks source link

RFE: basic infrastructure for tracking per-syscall/ABI kernel versions #381

Closed pcmoore closed 2 years ago

pcmoore commented 2 years ago

This commit adds basic support for tracking what kernel introduced a syscall for a given arch/ABI. It does not provide any of that kernel version information, leaving only a SCMP_KV_UNDEF placeholder, nor does it attempt to do anything meaningful with this new source of information; this patch simply establishes a new syscalls.csv format so that we can start properly recording the kernel versions.

Signed-off-by: Paul Moore paul@paul-moore.com

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 89.604% when pulling 0dfe82985941cef1bc2f2cbdd936ee18c428be0e on pcmoore:working-syscall into 832b65b0fe408f538cf84b361eb7261eb7243b00 on seccomp:main.

drakenclimber commented 2 years ago

Thanks, @pcmoore!!! I have built upon what you've done and populated the x86_64_kver column. See this branch.

I'll give it a more thorough code review later, but functionality-wise it seems to be working well.

pcmoore commented 2 years ago

Thanks, @pcmoore!!! I have built upon what you've done and populated the x86_64_kver column. See this branch.

Thanks for taking a quick look, let me know if anything pops up on closer inspection.

Also, thanks for all your work in putting together the scripting to determine the kernel versions. Looking at your branch, a few things come to mind:

Thoughts?

drakenclimber commented 2 years ago

Thanks for taking a quick look, let me know if anything pops up on closer inspection.

Will do.

Also, thanks for all your work in putting together the scripting to determine the kernel versions. Looking at your branch, a few things come to mind:

* We probably need to pick a "no earlier than" version where we simply won't go back any further if only to put a practical bound on the work involved in doing the initial population.  The earliest form of seccomp mode 2, `prctl(SECCOMP_MODE_FILTER)`, was introduced in Linux v3.5 so perhaps that is as good of an epoch as any.  We could also start with `SCMP_KV_3_5 = 100` to give us some wiggle room if we needed to go earlier at a later date.

Sounds good to me. I arbitrarily started with v5.4 because that's the base kernel of Oracle's UEK6. Going back to v3.5 makes more sense from seccomp's point of view. I'll make that change.

And the 100 enum trickery is a good idea.

* One of the main reasons why so much of the non-C libseccomp scripting is done in Bash is because I really dislike additional build dependencies, and Bash is pretty ubiquitous.  However, as I mentioned in the other thread, I think it may be okay here because I don't think we really need to check-in whatever tool we use to do the initial kver population; we run the tool to generate the CSV fields initially, then it's just a matter of making sure we add the kver fields when we add new syscalls.  It shouldn't be a problem.

Thanks. That's a really solid vision to only rely on Bash. I don't recall hearing you say that before.

arch-populate-version.py is definitely a one-off script that wouldn't be part of the ./autogen.sh && ./configure && make process, so I agree it doesn't need to meet the Bash requirement. I'm ambivalent whether we check it in or not.

I have a rough draft of a script that can turn the syscalls.csv versions into a C header file. It's currently in Python3, but I think it should be do-able to convert it to Bash.

Thanks!

drakenclimber commented 2 years ago

I incorporated your latest recommendations into my branch, @pcmoore. I think they were excellent improvements.

drakenclimber commented 2 years ago

@pcmoore, I noticed this oddity when running arch-syscall-validate. Not sure of the root cause:

$ ./arch-syscall-validate <kernel_src_path>
...
--- sh [library]
+++ sh [system]
@@ -1,4 +1,3 @@
-     0,0
 accept,344
 accept4,358
 access,33
drakenclimber commented 2 years ago

@pcmoore, I noticed this oddity when running arch-syscall-validate. Not sure of the root cause:

$ ./arch-syscall-validate <kernel_src_path>
...
--- sh [library]
+++ sh [system]
@@ -1,4 +1,3 @@
-     0,0
 accept,344
 accept4,358
 access,33

It's an easy fix. A newline was accidentally inserted into syscalls.csv on line 2. Removing that cleaned up the issue.

pcmoore commented 2 years ago

FYI, I just force pushed a slight change that moves the scmp_kver enum from include/syscalls.h.in to src/system.h; this should allow us work on this without potentially adding anything new to published API and potentially causing problems for us later. Once we have everything set we can move the definition back into include/syscalls.h.in.

drakenclimber commented 2 years ago

FYI, I just force pushed a slight change that moves the scmp_kver enum from include/syscalls.h.in to src/system.h; this should allow us work on this without potentially adding anything new to published API and potentially causing problems for us later. Once we have everything set we can move the definition back into include/syscalls.h.in.

Good call. Thanks, @pcmoore

pcmoore commented 2 years ago

Merged via 72198d99dd6fda40c1e6d0524caa61d82eddcf45, thanks for the review!