Open cions opened 3 weeks ago
Related Issues and Documentation
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
I find it weird that this crashes the process with SIGSYS. The way pidfd_open is implemented in linux it should catch the errno (ENOSYS):
The stacktrace shows it is crashing in runtime_entersyscall
. Perhaps there is a seccomp(2)
filter in place causing the process to receive a SIGSYS?
Yes, it would be extremely unfortunate if every unrecognized system call triggered a SIGSYS
signal. That would make it impossible to write programs that run on both older and newer kernel versions. We need to understand what is causing that SIGSYS
kernel. I don't think it is the kernel.
That said, I see that this is android. We may need to skip the pidfd calls on Android. CC @golang/android
Additionally, would be good to see a strace output to aid with debugging, i.e strace -f go env
.
I realized that the real culpit is seccomp.
$ grep Seccomp: /proc/self/status
Seccomp: 2
$ strace -fqq --signal=SIGSYS --trace=none go env
[pid 12717] --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0x5a46509700, si_syscall=__NR_pidfd_open, si_arch=AUDIT_ARCH_AARCH64} ---
So we should check if seccomp is enabled?
Checking if seccomp is enabled won't really help us, because we won't know the policy.
I think we should just skip the pidfd calls if GOOS == "android"
.
I think the safest approach is to disable pidfd on Android.
We can probably just make android use pidfd_other.go https://github.com/golang/go/blob/master/src/os/pidfd_other.go#L5
Alternatively, check kernel version? https://github.com/golang/go/blob/master/src/internal/syscall/unix/kernel_version_linux.go
It seems to me that the kernel version isn't going to tell us anything about the seccomp policy.
Change https://go.dev/cl/608518 mentions this issue: os: don't use pidfd functions on android
Not to check the seccomp policy, but to check if the kernel version supports pidfd. Since Android with newer kernel would not have the problem, I don't think disabling pidfd for GOOS=android is a good idea.
Is there reason to believe that the seccomp policy matches the kernel version?
In normal Linux use we don't have to check the kernel version, because the system call with fail with an ENOSYS
error. In this case the above discussion suggests that it is the Android seccomp policy that is sending the SIGSYS
signal. But the kernel might have been updated without updating the seccomp policy. Is there a way that we can find out?
https://github.com/termux/termux-packages/issues/21265 Users reported go works fine on newer Android
Thanks. I still don't know how to tell whether pidfd_open
is supported or not. I'm certainly happy to accept a patch that has been tested on multiple versions of Android.
Any chance this was a bug with android? I found this https://github.com/aosp-mirror/platform_bionic/commit/3de19151e508e14654a2d3204d9981c514f1c93a but it points to a private issue.
https://android-review.googlesource.com/c/platform/bionic/+/1208625 https://cs.android.com/android/_/android/platform/bionic/+/refs/tags/android-11.0.0_r1:libc/SECCOMP_WHITELIST_COMMON.TXT;l=76 pidfd_open was added to seccomp allow list since Android 11
Android 10 supports 3.18, 4.4, 4.9, 4.14, and 4.19 kernels
So, checking if the kernel version is 5.3 or newer before calling pidfd_open should work on all Android devices.
Oops, pidfd_send_signal was not allowed in Android 11, but it fixed in 12
Since Android 11 supports only 4.19 and 5.4 kernels (https://source.android.com/docs/core/architecture/kernel/android-common), change to check against 5.5 rather than 5.3
So, it's not the Android kernel but its seccomp policy which results in a process being killed (instead of something like returning ENOSYS). Apparently, this was fixed in Android 12, we can add a kludge to do a one-time runtime check for Android >= 12 and disable pidfd entirely if this requirement is not met, just to avoid being killed.
Alas I can't find any code that checks Android version in this repository.
This also means that there's no CI in place to test Android 11, or this would have been caught earlier.
You're right, we should check Android version >= 12 rather than kernel version.
Here is the way to get Android version in C (sorry I'm not familiar with Cgo) https://gist.github.com/cions/07fa5f11e38945fa96916888b7e88d0c
Thanks. If we have to we can add a call to __system_property_get
in runtime/cgo/gcc_android.c.
But honestly I think it would be simpler to just skip the pidfd calls on Android. Any patch to use them should be written by an Android developer who is able to test on various Android releases.
Change https://go.dev/cl/610515 mentions this issue: os: don't use pidfd on Android < 12
Go version
go version go1.23.0 android/arm64
Output of
go env
in your module/workspace:What did you do?
Just run
go env
(above output is by patched version)What did you see happen?
What did you expect to see?
According to https://go.dev/wiki/MinimumRequirements, Golang supports kernel version 2.6.32 or later, but
os.checkPidfd()
unconditionally callspidfd_open(2)
, which was introduced in 5.3.os.checkPidfd()
should check availability without calling potentially unavailable system calls. Alternatively, allow to disable the use of pidfd byGODEBUG
.Related: #62654 CC @kolyshkin