-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
os: os.checkPidfd() crashes with SIGSYS #69065
Comments
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): go/src/internal/syscall/unix/pidfd_linux.go Lines 18 to 21 in 6885bad
The stacktrace shows it is crashing in |
Yes, it would be extremely unfortunate if every unrecognized system call triggered a 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 |
I realized that the real culpit is seccomp.
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 |
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? |
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: |
Not to check the seccomp policy, but to check if the kernel version supports pidfd. |
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 |
termux/termux-packages#21265 |
Thanks. I still don't know how to tell whether |
Any chance this was a bug with android? I found this aosp-mirror/platform_bionic@3de1915 but it points to a private issue. |
https://android-review.googlesource.com/c/platform/bionic/+/1208625
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) |
Thanks. If we have to we can add a call to 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. |
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
Change https://go.dev/cl/610515 mentions this issue: |
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
Updates tailscale/tailscale#13452 Updates golang#69065 Signed-off-by: Brad Fitzpatrick <[email protected]>
Updates tailscale/tailscale#13452 Updates golang#69065 Signed-off-by: Brad Fitzpatrick <[email protected]>
Updates tailscale/tailscale#13452 Updates golang#69065 Signed-off-by: Brad Fitzpatrick <[email protected]>
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
Change https://go.dev/cl/614277 mentions this issue: |
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. Fixes golang#69065
@gopherbot Please open a backport to 1.23. This issue causes Go programs to be unable to exec other programs on earlier versions of Android. There is no workaround. |
Backport issue(s) opened: #69640 (for 1.23). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
Change https://go.dev/cl/616077 mentions this issue: |
In Android version 11 and earlier, pidfd-related system calls are not allowed by the seccomp policy, which causes crashes due to SIGSYS signals. For #69065 Fixes #69640 Change-Id: Ib29631639a5cf221ac11b4d82390cb79436b8657 GitHub-Last-Rev: aad6b3b GitHub-Pull-Request: #69543 Reviewed-on: https://go-review.googlesource.com/c/go/+/614277 Auto-Submit: Ian Lance Taylor <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: David Chase <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> (cherry picked from commit a3a05ed) Reviewed-on: https://go-review.googlesource.com/c/go/+/616077 Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Kirill Kolyshkin <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Mauri de Souza Meneguzzo <[email protected]> Auto-Submit: Michael Knyszek <[email protected]>
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
The text was updated successfully, but these errors were encountered: