The idea is to remove the need for cloning the entire runc binary by
replacing the final execve() call of the container process with an
execve() call to a clone of a small C binary which just does an execve()
of its arguments.
This provides similar protection against CVE-2019-5736 but without
requiring a >10MB binary copy for each "runc init". When compiled with
musl, runc-dmz is 13kB (though unfortunately with glibc, it is 1.1MB
which is still quite large).
It should be noted that there is still a window where the container
processes could get access to the host runc binary, but because we set
ourselves as non-dumpable the container would need CAP_SYS_PTRACE (which
is not enabled by default in Docker) in order to get around the
proc_fd_access_allowed() checks. In addition, since Linux 4.10[1] the
kernel blocks access entirely for user namespaced containers in this
scenario. For those cases we cannot use runc-dmz, but most containers
won't have this issue.
This new runc-dmz binary can be opted out of at compile time by setting
the "runc_nodmz" buildtag, and at runtime by setting the RUNC_DMZ=legacy
environment variable. In both cases, runc will fall back to the classic
/proc/self/exe-based cloning trick. If /proc/self/exe is already a
sealed memfd (namely if the user is using contrib/cmd/memfd-bind to
create a persistent sealed memfd for runc), neither runc-dmz nor
/proc/self/exe cloning will be used because they are not necessary.
[1]: https://github.com/torvalds/linux/commit/bfedb589252c01fa505ac9f6f2a3d5d68d707ef4
Co-authored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
[cyphar: address various review nits]
[cyphar: fix runc-dmz cross-compilation]
[cyphar: embed runc-dmz into runc binary and clone in Go code]
[cyphar: make runc-dmz optional, with fallback to /proc/self/exe cloning]
[cyphar: do not use runc-dmz when the container has certain privs]
Co-authored-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This allow us to remove the amount of C code in runc quite
substantially, as well as removing a whole execve(2) from the nsexec
path because we no longer spawn "runc init" only to re-exec "runc init"
after doing the clone.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix.
Thus, these annotations are no longer necessary. Hooray!
[1] https://github.com/polyfloyd/go-errorlint/pull/47
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When starting a new container, and the very last step of executing of a
user process fails (last lines of (*linuxStandardInit).Init), it is too
late to print a proper error since both the log pipe and the init pipe
are closed.
This is partially mitigated by using exec.LookPath() which is supposed
to say whether we will be able to execute or not. Alas, it fails to do
so when the binary to be executed resides on a filesystem mounted with
noexec flag.
A workaround would be to use access(2) with X_OK flag. Alas, it is not
working when runc itself is a setuid (or setgid) binary. In this case,
faccessat2(2) with AT_EACCESS can be used, but it is only available
since Linux v5.8.
So, use faccessat2(2) with AT_EACCESS if available. If not, fall back to
access(2) for non-setuid runc, and do nothing for setuid runc (as there
is nothing we can do). Note that this check if in addition to whatever
exec.LookPath does.
Fixes https://github.com/opencontainers/runc/issues/3520
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is not used since commit 5e7b48f7c0 (23 Mar 2017).
In case there are external users, they should switch to
opencontainers/selinux.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If the container binary to be run is removed in between runc create
and runc start, the latter spits the following error:
> can't exec user process: no such file or directory
This is a bit confusing since we don't see what file is missing.
Wrap the unix.Exec error into os.PathError, like in many other cases,
to provide some context. Remove the error wrapping from
(*linuxStandardInit).Init as it is now redundant.
With this patch, the error is now:
> exec /bin/false: no such file or directory
Reported-by: Daniel J Walsh <dwalsh@redhat.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin (3):
libct/system: add I and P process states
libct/system.Stat: fix/improve/speedup
libct/system/proc_test: fix, improve, add benchmark
LGTMs: thaJeztah cyphar
Go 1.17 introduce this new (and better) way to specify build tags.
For more info, see https://golang.org/design/draft-gobuild.
As a way to seamlessly switch from old to new build tags, gofmt (and
gopls) from go 1.17 adds the new tags along with the old ones.
Later, when go < 1.17 is no longer supported, the old build tags
can be removed.
Now, as I started to use latest gopls (v0.7.1), it adds these tags
while I edit. Rather than to randomly add new build tags, I guess
it is better to do it once for all files.
Mind that previous commits removed some tags that were useless,
so this one only touches packages that can at least be built
on non-linux.
Brought to you by
go1.17 fmt ./...
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Those states are available since Linux 4.14 (kernel commits
8ef9925b02c23e3838d5 and 06eb61844d841d003). Before this
patch, they were shown as unknown.
This is mostly cosmetical.
Note that I is described in /proc/pid/status as just "idle", although
elsewhere it says it's an idle kernel thread. Let's have it as "idle"
for brevity.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Remove PID field as it is useless.
2. Rewrite parseStat() to make it faster and more correct:
- do not use fmt.Scanf as it is very slow;
- avoid splitting data into 20+ fields, of which we only need 2;
- make sure to not panic on short lines and other bad input;
- add some bad input tests (some fail with old code);
- use LastIndexByte instead of LastIndex.
Benchmarks:
before (from the previous commit message):
> BenchmarkParseStat-4 116415 10804 ns/op
> BenchmarkParseRealStat-4 240 4781769 ns/op
after:
> BenchmarkParseStat-4 1164948 1068 ns/op
> BenchmarkParseRealStat-4 331 3458315 ns/op
We are seeing 10x speedup in a synthetic benchmark, and about 1.4x
speedup in a real world benchmark.
While at it, do not ignore any possible errors, and properly wrap those.
[v2: use pkg/errors more, remove t.Logf from test]
[v3: rebased; drop pkg/errors; gofumpt'ed]
[v4: rebased; improved description]
[v5: rebased; mention bad input tests, added second benchmark results]
[v6: remove PID field, do not use strings.Split, further speedup]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Add a test case that tests parentheses in command.
2. Replace individual comparisons with reflect.DeepEqual.
This also fixes wrong %-style types in Fatalf statements.
3. Replace Fatalf with Errorf so we don't bail out on the first
failure, and do not check result on error.
4. Add two benchmarks. On my laptop, they show:
BenchmarkParseStat-4 116415 10804 ns/op
BenchmarkParseRealStat-4 240 4781769 ns/op
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When running a script from an azure file share interrupted syscall
occurs quite frequently, to remedy this add retries around execve
syscall, when EINTR is returned.
Signed-off-by: Maksim An <maksiman@microsoft.com>
Errors from unix.* are always bare and thus can be used directly.
Add //nolint:errorlint annotation to ignore errors such as these:
libcontainer/system/xattrs_linux.go:18:7: comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
case errno == unix.ERANGE:
^
libcontainer/container_linux.go:1259:9: comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
if e != unix.EINVAL {
^
libcontainer/rootfs_linux.go:919:7: comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
if err != unix.EINVAL && err != unix.EPERM {
^
libcontainer/rootfs_linux.go:1002:4: switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors (errorlint)
switch err {
^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These were deprecated and moved; the stubs were included in the
last two (rc94, rc95) releases, so external consumers would have
the chance to update their code.
Removing this so that this doesn't get into v1.0.0 GA
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Most of these were false positives or cases where we want to ignore the
lint, but the change to the BPF generation is actually useful.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Moving these utilities to a separate package, so that consumers of this
package don't have to pull in the whole "system" package.
Looking at uses of these utilities (outside of runc itself);
`RunningInUserNS()` is used by [various external consumers][1],
so adding a "Deprecated" alias for this.
[1]: https://grep.app/search?current=2&q=.RunningInUserNS
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
GetProcessStartTime was deprecated over three Years ago in
439eaa3584, so we may as well remove
it now.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In many places (not all of them though) we can use `unix.`
instead of `syscall.` as these are indentical.
In particular, x/sys/unix defines:
```go
type Signal = syscall.Signal
type Errno = syscall.Errno
type SysProcAttr = syscall.SysProcAttr
const ENODEV = syscall.Errno(0x13)
```
and unix.Exec() calls syscall.Exec().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the following compilation error on 32bit ARM:
```
$ GOARCH=arm GOARCH=6 go build ./libcontainer/system/
libcontainer/system/linux.go:119:89: constant 4294967295 overflows int
```
Signed-off-by: Tibor Vass <tibor@docker.com>
When a subreaper is enabled, it might expect to reap a process and
retrieve its exit code. That's the reason why this patch is giving
the possibility to define the usage of a subreaper as a consumer of
libcontainer. Relying on this information, libcontainer will not
wait for signalled processes in case a subreaper has been set.
Fixes#1677
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
runc is not supported on FreeBSD, so remove all FreeBSD specific bits.
As suggested by @crosbymichael in #1653
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
There are essentially two possible implementations for Setuid/Setgid on
Linux, either using SYS_SETUID32/SYS_SETGID32 or SYS_SETUID/SYS_SETGID,
depending on the architecture (see golang/go#1435 for why Setuid/Setgid
aren currently implemented for Linux neither in syscall nor in
golang.org/x/sys/unix).
Reduce duplication by merging the currently implemented variants and
adjusting the build tags accordingly.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Use IoctlGetInt and IoctlGetTermios/IoctlSetTermios instead of manually
reimplementing them.
Because of unlockpt, the ioctl wrapper is still needed as it needs to
pass a pointer to a value, which is not supported by any ioctl function
in x/sys/unix yet.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Use unix.Prctl() instead of manually reimplementing it using
unix.RawSyscall. Also use unix.SECCOMP_MODE_FILTER instead of locally
defining it.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
And Stat_t.PID and Stat_t.Name while we're at it. Then use the new
.State property in runType to distinguish between running and
zombie/dead processes, since kill(2) does not [1]. With this change
we no longer claim Running status for zombie/dead processes.
I've also removed the kill(2) call from runType. It was originally
added in 13841ef3 (new-api: return the Running state only if the init
process is alive, 2014-12-23), but we've been accessing
/proc/[pid]/stat since 14e95b2a (Make state detection precise,
2016-07-05, #930), and with the /stat access the kill(2) check is
redundant.
I also don't see much point to the previously-separate
doesInitProcessExist, so I've inlined that logic in runType.
It would be nice to distinguish between "/proc/[pid]/stat doesn't
exist" and errors parsing its contents, but I've skipped that for the
moment.
The Running -> Stopped change in checkpoint_test.go is because the
post-checkpoint process is a zombie, and with this commit zombie
processes are Stopped (and no longer Running).
[1]: https://github.com/opencontainers/runc/pull/1483#issuecomment-307527789
Signed-off-by: W. Trevor King <wking@tremily.us>
Use the symlink xattr syscall wrappers Lgetxattr, Llistxattr and
Lsetxattr from x/sys/unix (introduced in
golang/sys@b90f89a1e7) instead of
providing own wrappers. Leave the functionality of system.Lgetxattr
intact with respect to the retry with a larger buffer, but switch it to
use unix.Lgetxattr.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Since syscall is outdated and broken for some architectures,
use x/sys/unix instead.
There are still some dependencies on the syscall package that will
remain in syscall for the forseeable future:
Errno
Signal
SysProcAttr
Additionally:
- os still uses syscall, so it needs to be kept for anything
returning *os.ProcessState, such as process.Wait.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com>
The original SETUID takes a 16 bit UID. Linux 2.4 introduced a new
syscall, SETUID32, with support for 32 bit UIDs. The setgid wrapper
already uses SETGID32.
Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
Revert: #935Fixes: #946
I can reproduce #946 on some machines, the problem is on
some machines, it could be very fast that modify time
of `memory.kmem.limit_in_bytes` could be the same as
before it's modified.
And now we'll call `SetKernelMemory` twice on container
creation which cause the second time failure.
Revert this before we find a better solution.
Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
lifubang