x/sys/unix: Sigset_t follows libc instead of kernel definition on Linux

[lmb]

Sigset_t in x/sys/unix is defined as follows for amd64:

type Sigset_t struct {
	Val [16]uint64
}

In total, it has space for 1024 signals. However, Linux syscalls expect a sigset_t like so:

#define _NSIG		64

#define _NSIG_BPW	64

#define _NSIG_WORDS	(_NSIG / _NSIG_BPW)

typedef struct {
	unsigned long sig[_NSIG_WORDS];
} sigset_t;

This becomes a problem for syscalls that require sizeof(sigset_t) to be passed, for example rt_sigprocmask:

int syscall(SYS_rt_sigprocmask, int how, const kernel_sigset_t *set,
                       kernel_sigset_t *oldset, size_t sigsetsize)

Passing unsafe.Sizeof(unix.Sigset_t{}) will return EINVAL.

This stackoverflow answer suggests that the discrepancy is due to glibc defining a larger than necessary sigset_t: https://unix.stackexchange.com/a/399356

From my POV, x/sys/unix should follow the kernel layout, not the libc one. I think this hasn't been a problem so far since all existing users of Sigset_t only take a pointer, so a larger than necessary structure is not a problem. Is there something that would break if we changed the definition?

[lmb]

The runtime has https://github.com/golang/go/blob/d11c58eedbee29b4912dd50508b36ad15ebb739e/src/runtime/os_linux_generic.go and friends to deal with this. Maybe it makes sense to just lift that code into sys/unix?

[cherrymui]

cc @ianlancetaylor @bradfitz @tklauser

[ianlancetaylor]

I'm not sure there is a good answer here. What should happen if the kernel changes the number of signals?. Arguably people calling unix.Syscall(unix.SYS_RT_SIGPROGMASK, ...) need to know what they are doing, and not try to use Sigset_t.

[lmb]

I'm not sure there is a good answer here. What should happen if the kernel changes the number of signals?

I think that would only work if the kernel started to support a sigset_t smaller than the in kernel sigset_t at the same time. Otherwise user space would break, which is against Linux policy.

From the sys/unix perspective this would mean making the struct larger and recompiling.

[lmb]

Maybe we can copy what was done for Signalfd? I could add RtSigprocmask() like so:

https://cs.opensource.google/go/x/sys/+/fb04ddd9:unix/syscall_linux.go;l=1944

We omit the size argument from the wrapper and pass the size ourselves. Passing NSIG/8 is kind of a hack but I guess it works well enough, and this avoids changing sigset_t.

Maybe in addition export NSIG so that implementations of sigaddset can return an error if trying to set an unsupported signal?

[ianlancetaylor]

I don't see why userspace would break if the kernel increased the number of signals. As far as I know the reason that the size is passed to the system call is to permit the kernel to increase the signal count. The kernel would simply continue to accept the smaller size.

[ianlancetaylor]

I agree that if add Sigprocmask then it ought to work like Signalfd. And perhaps that is the way to go. Normally the Go runtime should be in charge of the signal mask, but I suppose there are special cases where it is helpful for Go code to control it temporarily. Actually it shouldn't be Sigprocmask, it should be Pthread_sigmask, although I think they may be the same on Linux.

I don't see a reason to export NSIG, that just seems to set us up for problems in the unlikely event that it ever changes. Note that just recompiling with a new version of x/sys/unix doesn't really help because programs would need to run on both old and new kernels.

[lmb]

The kernel would simply continue to accept the smaller size.

Yes, that's what I was trying to say. I suspect we're talking past each other with this sigset_t size stuff, but adding a rt_sigprocmask wrapper would solve my problem without having to dive deeper here :)

I suppose there are special cases where it is helpful for Go code to control it temporarily.

Yes, we need it for #55243.

Actually it shouldn't be Sigprocmask, it should be Pthread_sigmask, although I think they may be the same on Linux.

This would be the first Pthread* syscall to be exported so I did some digging:

• For Linux/glibc pthread_sigmask != sigprocmask due to blocking of some signals.
• For Linux/musl the two are pretty much equivalent, modulo some return value massaging I don't understand.
• On Darwin, pthread_sigmask actually calls a syscall __pthread_syscall.

Is it OK if I add PthreadSigmask for Linux only to get started?

[ianlancetaylor]

According to POSIX sigprocmask is not defined in a multi-threaded process, so using sigprocmask seems clearly wrong.

It's fine to add PthreadSigmask for Linux only as long as the API can in principle be implemented on any Unix system. Thanks.

[gopherbot]

Change https://go.dev/cl/434555 mentions this issue: unix: add PthreadSigmask

[gopherbot]

Change https://go.dev/cl/435095 mentions this issue: unix: add PthreadSigmask for Linux

[cherrymui]

I think the name PthreadSigmask is a bit confusing. It sounds like it is using the pthread library (the C library), but it doesn't?

[lmb]

I found it confusing too, but I believe it's the correct name. pthread is a standard, not a library: https://man7.org/linux/man-pages/man7/pthreads.7.html Some of pthreads can be implemented in user space, some of it in kernel space (see my links for Darwin where pthread_sigmask is an actual syscall). There are even distinct implementations on Linux of pthreads, for example nptl and musl.

[ianlancetaylor]

pthread_sigmask is the POSIX name (https://pubs.opengroup.org/onlinepubs/009696899/functions/pthread_sigmask.html). I agree that it's not a great name but I don't know of a better one.

[cherrymui]

Okay, thanks. Then I guess PthreadSigmask is probably okay.

[gopherbot]

Change https://go.dev/cl/435775 mentions this issue: unix: add SIG_BLOCK and friends for Linux

[lmb]

Sorry for sending another CL, I hadn't realised that the SIG_* constants used by PthreadSigmask use arch specific values.

[lmb]

Thanks very much everyone!