[supervisor] execve into ring3 #2664
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csweichel
force-pushed
the
cw/supervisor-execve-ring3
branch
3 times, most recently
from
28a182c to
f511f1a
Compare
|
/werft run 👍 started the job as gitpod-build-cw-supervisor-execve-ring3.23 |
geropl
reviewed
Jan 8, 2021
components/supervisor/cmd/rings.go
Outdated
| log.WithError(err).Error("failed to start the child process") | ||
| err = cap.SetUID(33333) | ||
| if err != nil { | ||
| log.WithError(err).Error("cannot setgid") |
geropl
reviewed
Jan 8, 2021
components/supervisor/cmd/rings.go
Outdated
| }, | ||
| err = cap.SetGroups(33333) | ||
| if err != nil { | ||
| log.WithError(err).Error("cannot setuid") |
geropl
approved these changes
Jan 8, 2021
csweichel
force-pushed
the
cw/supervisor-execve-ring3
branch
2 times, most recently
from
1a9c380 to
fd3db95
Compare
csweichel
force-pushed
the
cw/supervisor-execve-ring3
branch
from
fd3db95 to
65dc59c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes ring2 execve
supervisor run(aka ring3). This way PID 1 in the workspace issupervisor run, much like it would be without user namespaces. Hence supervisor can properly do its job as reaper, and the graceful shutdown of tasks is a bit less brittle (#2654).How does it work?
Instead of starting a child process, ring2 now calls setgid/setuid to drop to the Gitpod user (
33333) and then execve's/proc/self/exe run, thereby replacing the ring2 process with supervisor run.Why didn't we do this before?
setuid/setgid have a long and difficult history in Go. Go 1.16 introduced
AllThreadsSyscallwhich makes setuid/setgid work on all threads without the use of CGO. TLDR; Before Go 1.16 we'd need CGO, but we don't want CGO because supervisor has to run in glibc and muslibc environments. cap - a Go-native port of libcap - supports this feature, and if it's present supports capability-preservingsetuid/setgidcalls without CGO.But wait, Go 1.16 is still in beta?!?
Indeed. That's why leeway now supports choosing a Go version on a package level. Starting with this PR, we compile supervisor with go1.16beta1, and the rest using the Go version we have installed in the dev-environment image. Once Go 1.16 is out of beta (Feb 2021), we'll switch over for all components. Go betas have been really rather stable and I don't expect issues because we build supervisor using Go 1.16. Note golang/go#43149 does not concern us, because only ring2 uses AllThreadsSyscall and ring2 does not use
signal.How to test
proper-shutdown.txtin the list of changed files. This file was created when the task stopped properly../zombie & sleep 1 && ps guaxf | grep zombie... once the zombie parent exitsps guaxf | grep defunctshould not return anything. Previously, the defunct process would still have been around.Screen.Recording.2021-01-05.at.11.22.18.mov
/werft https