signalfd: Add native support for Linux's signalfd The signalfd system call handles signals using a special signalfd file descriptor. The caller may specify a mask of signals for signalfd to catch. When the desciptor is read, it will return a signalfd_siginfo structure populated with signal information from the sigqueue, similar to sigwaitinfo(2). The signalfd descriptor maintains traditional file descriptor semantics, so it may be passed to other processes, preserved across fork(2), and monitored via kevent(2), poll(2), or select(2). This signalfd interface is based on the behavior of Linux's signalfd implementation. It was designed to be drop-in compatible with existing Linux signalfd code.
Details
- Reviewers
markj imp kib val_packett.cool brooks
This implementation passes all of epoll-shim's signalfd tests:
https://github.com/jiixyj/epoll-shim/blob/master/test/signalfd-test.c
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
This has come up in the context of Linux-compatibility jails with musl libc distributions. Could someone please review this?
For example in context:
Feb 17 18:51:12 myhost kernel: linux: jid 7 pid 54884 (node): syscall membarrier not implemented
Feb 17 18:57:33 myhost kernel: linux: jid 8 pid 56301 (postgres): syscall signalfd4 not implemented
Feb 17 18:57:33 myhost kernel: linux: jid 8 pid 56301 (postgres): syscall signalfd not implemented
| sys/kern/sys_signalfd.c | ||
|---|---|---|
| 97 | These would only be for SIGCHLD and would come from the rusage for the child process. If we wanted this, we would just need to extend ksiginfo_t to add those fields and then pass the full ksi to this function. We could also perhaps extend our native siginfo_t to pass those fields for SIGCHLD as well. We do have room in the existing union to add these fields (which would perhaps be the simplest path forward). That could be done as a separate followup even. | |
| 99 | Hmm, I can't find documentation for ssi_call_addr or ssi_arch and what they are supposed to even be. | |
| 103 | Does Linux only provide this for SIGBUS? On FreeBSD we provide this for various signals (the claim I wrote in siginfo(3) is that this is set for any synchronous signal). I think you could just assign this unconditionally. | |
| sys/sys/signalfd.h | ||
| 64 | This should probably be a _Static_assert() | |
| lib/libc/sys/Symbol.map | ||
|---|---|---|
| 423 | The 1.7 namespace is sealed. | |
| sys/kern/kern_sig.c | ||
| 2297 | So this 'else if' breaks linuxolator which would never wake up target if there is a single signalfd. | |
| sys/kern/sys_signalfd.c | ||
| 59 | Blank line between vars and statements. p must be locked, this must be asserted in the function. | |
| 62 | What is the purpose of taking the sfd lock there? | |
| 73 | Same notes as for signalfd_post() | |
| 90 | This line is clear example of why we do not design (native) FreeBSD API with fixed-sized integers. | |
| 150 | This causes random EFAULTs which are not justified. uiomove_nofault() is not for such use. | |
| 336 | sfd leaked | |
| 352 | Unlocked modification of fp->f_flag | |
| sys/sys/signalfd.h | ||
| 34 | This gives huge and unjustifiable user compilation environment namespace pollution. | |
| 42 | The structure member types are explicitly not how FreeBSD defines user-visible API. We do not define interfaces in terms of fixed integer sizes. | |
| sys/kern/sys_signalfd.c | ||
|---|---|---|
| 50 | What is the purpose of the sfd_lock? What should it protect? You declare that p_sfd is locked with the process lock. Then in file methods, you always use curproc lock to lock the list. But what happen if the signalfd is passed over unix socket? You marked signalfd as passable, and I suspect that Linux does allow to send signalfds over local sockets. | |