Skip to content
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.

Sign up for GitHub

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

Jump to bottom

runtime: killed threads do not crash whole process #30753

Open
bobrik opened this issue Mar 11, 2019 · 8 comments
Open

runtime: killed threads do not crash whole process #30753

bobrik opened this issue Mar 11, 2019 · 8 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@bobrik
Copy link

bobrik commented Mar 11, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ivan/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ivan/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build580235778=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have a program running under systemd and with SystemCallFilter applied. My list of allowed syscalls does not include madvise, which eventually leads to:

$ sudo strace -f -p $(pidof tracefwdr) 2>&1 | fgrep SIGSYS -C5
[pid 125587] <... futex resumed> )      = 0
[pid 125588] <... read resumed> "\202\201\0\temitBatch\34\34\30\10nginx-fl\31<\30\16jae"..., 65000) = 7271
[pid 125587] madvise(0xc000400000, 2097152, MADV_NOHUGEPAGE <unfinished ...>
[pid 125588] read(3, 0xc000438000, 65000) = -1 EAGAIN (Resource temporarily unavailable)
[pid 125587] <... madvise resumed>)     = ?
[pid 125587] +++ killed by SIGSYS +++
[pid 125588] sched_yield()              = 0

Here one thread is killed by SIGSYS for seccomp violation.

What did you expect to see?

Whole process panics, clearly indicating failure.

What did you see instead?

One thread is dead and random part of my program is not working anymore (either reads from channel or udp socket, not so sure).
@bradfitz bradfitz changed the title Killed threads do not crash whole process runtime: killed threads do not crash whole process Mar 11, 2019
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 11, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Mar 11, 2019
@bradfitz
Copy link
Contributor

/cc @aclements @ianlancetaylor

@ianlancetaylor
Copy link
Member

I don't know what we can do here. I'm open to suggestions.

@bobrik
Copy link
Author

bobrik commented Mar 12, 2019

If it's not possible to detect silently crashed / killed threads from Go runtime, I guess I can ask systemd to implement SECCOMP_RET_KILL_PROCESS as an option or return an errno:

       SECCOMP_RET_KILL_PROCESS (since Linux 4.14)
              This value results in immediate termination of the process,
              with a core dump.  The system call is not executed.  By con‐
              trast with SECCOMP_RET_KILL_THREAD below, all threads in the
              thread group are terminated.  (For a discussion of thread
              groups, see the description of the CLONE_THREAD flag in
              clone(2).)

Feel free to close if it's not possible.

@ianlancetaylor
Copy link
Member

Thanks. I don't know that it's not possible. I just don't know of a way to do it.

@bobrik bobrik mentioned this issue Mar 12, 2019
Closed
@aclements
Copy link
Member

Killing just a thread like this seems like it would break all sorts of things, not just Go. Is this really standard behavior for systemd?

I'm also confused because your strace output indicates a SIGSYS, but Go is supposed to trap SIGSYS and turn it into a fatal runtime panic. Does seccomp somehow produce a SIGSYS that can't actually be trapped?

@bobrik
Copy link
Author

bobrik commented Mar 12, 2019

Yes, by default seccomp's SIGSYS is untrappable:

              The process terminates as though killed by a SIGSYS signal.
              Even if a signal handler has been registered for SIGSYS, the
              handler will be ignored in this case and the process always
              terminates.  To a parent process that is waiting on this
              process (using waitpid(2) or similar), the returned wstatus
              will indicate that its child was terminated as though by a
              SIGSYS signal.

It can be trapped if SCMP_ACT_TRAP is used:

       SCMP_ACT_TRAP
              The thread will be sent a SIGSYS signal when it calls a
              syscall that does not match any of the configured seccomp
              filter rules.  It may catch this and change its behavior
              accordingly.  When using SA_SIGINFO with sigaction(2), si_code
              will be set to SYS_SECCOMP, si_syscall will be set to the
              syscall that failed the rules, and si_arch will be set to the
              AUDIT_ARCH for the active ABI.

I agree that killing a thread seems a bit weird in context of systemd and mentioned it here:

@dqminh
Copy link

dqminh commented Mar 12, 2019

So currently it looks like systemd can do 2 things when it comes to not allowed syscalls ( more detailed explanation can be found at https://www.freedesktop.org/software/systemd/man/systemd.exec.html#System%20Call%20Filtering:

  • by default, it does SCMP_ACT_KILL, which does:
              The thread will be terminated by the kernel with SIGSYS when
              it calls a syscall that does not match any of the configured
              seccomp filter rules.  The thread will not be able to catch
              the signal

This doesn't look like what we want ever.

  • When armed with SystemCallErrorNumber, it returns the error number instead, which maybe what we want here.

One interesting thing i noted out of this is that we call madvise without checking return code ( 295a4d8 ), so returning EPERM errors doesnt signal failures in those cases. Should we do more thorough checks for madvise ? I'm not sure if there are any other occurrences.

@ianlancetaylor
Copy link
Member

I don't think it matters if madvise fails. The program won't return memory to the OS but will otherwise run unchanged.

@bobrik bobrik mentioned this issue Mar 12, 2019
Closed
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@mknyszek mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Backlog
Development

No branches or pull requests
8 participants
@bradfitz @dqminh @bobrik @rsc @andybons @aclements @ianlancetaylor @gopherbot