mirror of
https://github.com/netbirdio/gvisor.git
synced 2026-05-22 17:12:49 -07:00
Move rlimit-setting code from runsc main to run when starting a sandbox.
This allows `runsc` subcommands that don't start sandboxes to run in restricted contexts without printing a warning about not being able to set `RLIMIT_MEMLOCK` when the ability to do so this doesn't matter. In particular, this helps with `runsc metric-server`, which can be locked down to run with very little capabilities. A previous version of this change had moved this to the beginning of the `runsc boot` subcommand code. However, this doesn't work, because `runsc boot` runs as an unprivileged user (`nobody`) and does not have `CAP_SYS_RESOURCE`. Prior to that change, all `runsc` invocations tried to call `setrlimit`, so what happened in practice is that `runsc create` (running as `root`) would call `setrlimit`, and then `runsc boot` would inherit the `RLIM_INFINITY` and would therefore never actually call `setrlimit` by itself. When moving the `setrlimit` code to only run within `runsc boot`, suddenly the `runsc boot` invocation found itself in a context where it started trying to call `setrlimit`, which would silently fail. This approach has the downside of having the side-effect of needlessly setting `RLIM_INFINITY` on the calling `runsc` process. This was effectively what was already happening prior to moving this code into `runsc boot` anyway, so this should be OK. The alternative would be to add yet another intermediate subcommand before `runsc boot` which runs with `CAP_SYS_RESOURCE`, then calls `setrlimit`, then drops `CAP_SYS_RESOURCE`, then execs `runsc boot`, but that seems like adding a lot more extra complexity to the boot process than is warranted for this feature. Thanks to Ayush Ranjan for bisecting the performance regression down to this change. Ran benchmarks and performance is comparable to before moving `setrlimit` code within `runsc boot`. PiperOrigin-RevId: 521916084
This commit is contained in:
committed by
gVisor bot
parent
a699bc8a39
commit
8f991198b4
@@ -250,23 +250,6 @@ func Main() {
|
||||
}
|
||||
linux.SetAFSSyscallPanic(conf.TestOnlyAFSSyscallPanic)
|
||||
|
||||
// pgalloc.MemoryFile (which provides application memory) sometimes briefly
|
||||
// mlock(2)s ranges of memory in order to fault in a large number of pages at
|
||||
// a time. Try to make RLIMIT_MEMLOCK unlimited so that it can do so. runsc
|
||||
// expects to run in a memory cgroup that limits its memory usage as
|
||||
// required.
|
||||
var rlim unix.Rlimit
|
||||
if err := unix.Getrlimit(unix.RLIMIT_MEMLOCK, &rlim); err != nil {
|
||||
log.Warningf("Failed to get RLIMIT_MEMLOCK: %v", err)
|
||||
} else if rlim.Cur != unix.RLIM_INFINITY || rlim.Max != unix.RLIM_INFINITY {
|
||||
rlim.Cur = unix.RLIM_INFINITY
|
||||
rlim.Max = unix.RLIM_INFINITY
|
||||
if err := unix.Setrlimit(unix.RLIMIT_MEMLOCK, &rlim); err != nil {
|
||||
// We may not have CAP_SYS_RESOURCE, so this failure may be expected.
|
||||
log.Infof("Failed to set RLIMIT_MEMLOCK: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Call the subcommand and pass in the configuration.
|
||||
var ws unix.WaitStatus
|
||||
subcmdCode := subcommands.Execute(context.Background(), conf, &ws)
|
||||
|
||||
@@ -612,6 +612,28 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn
|
||||
donations := donation.Agency{}
|
||||
defer donations.Close()
|
||||
|
||||
// pgalloc.MemoryFile (which provides application memory) sometimes briefly
|
||||
// mlock(2)s ranges of memory in order to fault in a large number of pages at
|
||||
// a time. Try to make RLIMIT_MEMLOCK unlimited so that it can do so. runsc
|
||||
// expects to run in a memory cgroup that limits its memory usage as
|
||||
// required.
|
||||
// This needs to be done before exec'ing `runsc boot`, as that subcommand
|
||||
// runs as an unprivileged user that will not be able to call `setrlimit`
|
||||
// by itself. Calling `setrlimit` here will have the side-effect of setting
|
||||
// the limit on the currently-running `runsc` process as well, but that
|
||||
// should be OK too.
|
||||
var rlim unix.Rlimit
|
||||
if err := unix.Getrlimit(unix.RLIMIT_MEMLOCK, &rlim); err != nil {
|
||||
log.Warningf("Failed to get RLIMIT_MEMLOCK: %v", err)
|
||||
} else if rlim.Cur != unix.RLIM_INFINITY || rlim.Max != unix.RLIM_INFINITY {
|
||||
rlim.Cur = unix.RLIM_INFINITY
|
||||
rlim.Max = unix.RLIM_INFINITY
|
||||
if err := unix.Setrlimit(unix.RLIMIT_MEMLOCK, &rlim); err != nil {
|
||||
// We may not have CAP_SYS_RESOURCE, so this failure may be expected.
|
||||
log.Infof("Failed to set RLIMIT_MEMLOCK: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
//
|
||||
// These flags must come BEFORE the "boot" command in cmd.Args.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user