-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFE: extend SCMP_FLTATR_API_SYSRAWRC support to the notification API #302
Comments
Hi @alban. Look at
We force |
I'm going to close this issue as NOTABUG, but feel free to continue to discuss this here if you have additional questions. We can reopen if it looks like there is a real problem that needs to be addressed. |
Hi @pcmoore, thanks for the quick reply.
But the new functions
are using |
Ah ... sorry about that. Actually, it goes deeper than the API level, look in "system.c" and you'll see that the libseccomp system level calls don't propogate those up to the API level functions. Looking at the manpages, it appears we don't make any claims about I'll go ahead and reopen this as a RFE since we don't currently make any claims about this working. @drakenclimber I think this is v2.6.x (possibly v2.5.2) material as I don't want to slow the v2.5.1 release. If you think otherwise let me know. |
seccomp/libseccomp#302 Signed-off-by: Alban Crequy <[email protected]>
Thanks! For the record, I fount the workaround in the Golang side to be able distinguish ENOENT and EINTR, with this code pattern: - if retCode := C.seccomp_notify_respond(C.int(fd), resp); retCode != 0 {
- return errRc(retCode)
+ for {
+ retCode, errno := C.seccomp_notify_respond(C.int(fd), resp)
+ if errno == syscall.EINTR {
+ continue
+ }
+ if errno == syscall.ENOENT {
+ return errno
+ }
+ if retCode != 0 {
+ return errRc(retCode)
+ }
+ break
} With this, it seems to work fine with libseccomp-2.5.0, |
This commit adds the seccomp userspace notification API present in version >= 2.5.0 of the libseccomp library. This API allows userspace to get a notification when a filter configured with a notification action triggers. The trigger suspends processing of the syscall until the notification is delivered to userspace and acknowledged back. To support the implementation, the following changes were necessary: - Added package init function to ensure libseccomp is properly initialized. It calls GetApi() in order to initialize the cgo libseccomp API level. This is necessary in order for libseccomp to properly handle other libseccomp APIs. - Fix errors reported by go vet such as "can't check non-constant format in call to Sprintf" This patch includes updated test updates for the new feature: - The Travis CI pipeline was previously running the tests on libseccomp from Ubuntu Bionic. This patch adds a matrix to test on various libseccomp versions (2.2.1, 2.4.4, 2.5.0). This is to check we don't break compatibility with older versions and return errors appropriately when running on an old version without seccomp notify support. This is necessary for downstream projects like runc that keeps support for CentOS 7. This uses PKG_CONFIG_PATH and LD_LIBRARY_PATH to compile and run the tests with different versions of libseccomp installed in a prefix. This also splits 'make check' into two separate 'make' commands, so that the tests run even if the vet fails. - Introduce execInSubprocess to run each test in a new process. This is because the kernel does not allow to remove a seccomp filters from a process, so a process cannot be reused for a subsequent test. Logs from subprocesses are read from the parent process and printed with the appropriate indentation. - Fix seccomp TestLogAct. This test was written in such as way that once the filter was loaded, it blocked almost all system calls, thereby making disabling the filter impossible and sometimes causing the Go runtime to fail to allocate memory. This fix simplifies the test and fixes these isuses. - Test timeout is reduced to a reasonable limit to help detect freezes as explained in the Makefile. - The main test for seccomp notification mechanism: TestNotif The test works with a couple of goroutines. One goroutine configures a seccomp filter with the notification action and generates syscalls that trigger the action. The other goroutine acts as a notifcation handler, verifies that the notification received from the kernel is correct, and generates an appropriate response. - An additional test for seccomp notification when it is not supported: TestNotifUnsupported. This gets tested on older kernels or with older libseccomp. - Handle the case where libseccomp returns EINTR or ENOENT, as reported here: seccomp/libseccomp#302. This patch is based on initial work in PR 50 by: - Cesar Talledo <[email protected]> - Rodny Molina <[email protected]> Co-authored-by: Rodrigo Campos <[email protected]> Signed-off-by: Alban Crequy <[email protected]> Signed-off-by: Rodrigo Campos <[email protected]> Acked-by: Tom Hromatka <[email protected]> Signed-off-by: Paul Moore <[email protected]>
The go runtime can interrupt the program quite frequently with SIGURG (this is exacerbated in go >= 1.14). For that reason, if these C function return code is not 0 and errno is set to EINTR, we run them again transparently for the user. Also, it is possible that errno is set to ENOENT by the ioctl() syscall done underneath, and what this means is that the notification is not valid anymore (the kernel sets that). For users of this library to know when this notification is not valid anymore, we return it in this case too. This was useful for us to properly react to different failures in our seccomp agent in golang. To do this we can't rely on the return code from libseccomp, and need to check the errno, because libseccomp returns fixed codes for errors. Like, for example here[1], but it does this for the other functions changed here too. These issues were reported to libseccomp here: seccomp/libseccomp#302. [1]: https://github.com/seccomp/libseccomp/blob/main/src/system.c#L501-L502 Signed-off-by: Rodrigo Campos <[email protected]> Acked-by: Tom Hromatka <[email protected]> Signed-off-by: Paul Moore <[email protected]>
As I sat down to implement this request I immediately ran into a problem: the three APIs mentioned above do not take a libseccomp filter handle and as a result we have no way to query for a Thoughts? @alban, is your golang workaround/fix still working okay for you? |
Good sleuthing @pcmoore, and that's a bummer about the API. Sounds like we need a place to record API improvements/upgrades. This would help us not forget things, and it may help us make more future-proof APIs. If @alban is cool with it, I'm fine with marking this as WONTFIX. |
To be honest, I don't remember, I don't think I've used the API since I've filed the issue. So resolving as wontfix is fine for me. Thanks for digging into the issue! |
The implementation of
seccomp_notify_receive()
just uses:Regardless of the error in
errno
, it returns-ECANCELED
.When debugging with strace, I found out that the ioctl returned EINTR because of the signal SIGURG:
The signal SIGURL is generated by the Golang runtime, see details in https://go-review.googlesource.com/c/go/+/232862/
According to that report, the syscall should just be reissued when getting EINTR.
Ideally libseccomp should take care of that, so it it transparent for users.
As a workaround, I tried to reissue the syscall by calling seccomp_notify_receive() in my program, and to do the same in
seccomp_notify_respond()
because it has the same pattern:But sometimes I get the return value errno = ENOENT:
libseccomp does not allow me to distinguish between ENOENT (the target process has been interrupted just before calling
SECCOMP_IOCTL_NOTIF_SEND
) and EINTR (the seccomp agent was interrupted by the SIGURG signal).Additionally, I cannot read
errno
from Golang, so I cannot have a workaround.The text was updated successfully, but these errors were encountered: