Page MenuHomeFreeBSD

signalfd: Add native support for Linux's signalfd
Needs ReviewPublic

Authored by jfree on Jun 28 2023, 7:52 PM.
Tags
None
Referenced Files
F162114797: D40791.diff
Thu, Jul 9, 10:24 PM
Unknown Object (File)
Wed, Jul 8, 11:30 PM
Unknown Object (File)
Wed, Jul 1, 11:41 PM
Unknown Object (File)
Wed, Jul 1, 11:18 PM
Unknown Object (File)
Fri, Jun 26, 11:54 AM
Unknown Object (File)
Thu, Jun 25, 10:08 AM
Unknown Object (File)
Tue, Jun 23, 3:49 AM
Unknown Object (File)
Sat, Jun 20, 8:50 AM

Details

Summary
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.
Test Plan

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

jfree requested review of this revision.Jun 28 2023, 7:52 PM

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

jhb added inline comments.
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.
FWIW most of these headers are not even needed for kernel, even in the current header form.

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.