A regression reported for runc v1.1.3 says that "runc exec -t" fails
after doing "systemctl daemon-reload":
> exec failed: unable to start container process: open /dev/pts/0: operation not permitted: unknown
Apparently, with commit 7219387eb7 we are no longer adding
"DeviceAllow=char-pts rwm" rule (because os.Stat("char-pts") returns
ENOENT).
The bug can only be seen after "systemctl daemon-reload" because runc
also applies the same rules manually (by writing to devices.allow for
cgroup v1), and apparently reloading systemd leads to re-applying the
rules that systemd has (thus removing the char-pts access).
The fix is to do os.Stat only for "/dev" paths.
Also, emit a warning that the path was skipped. Since the original idea
was to emit less warnings, demote the level to debug.
Note this also fixes the issue of not adding "m" permission for block-*
and char-* devices.
A test case is added, which reliably fails before the fix
on both cgroup v1 and v2.
Fixes: https://github.com/opencontainers/runc/issues/3551
Fixes: 7219387eb7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since Go 1.19, godoc recognizes lists, code blocks, headings etc. It
also reformats the sources making it more apparent that these features
are used.
Fix a few places where it misinterpreted the formatting (such as
indented vs unindented), and format the result using the gofumpt
from HEAD, which already incorporates gofmt 1.19 changes.
Some more fixes (and enhancements) might be required.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Unless the container's runtime config has intelRdt configuration set,
any checks for whether Intel RDT is supported or the resctrl filesystem
is mounted are a waste of time as, per the OCI Runtime Spec, "the
runtime MUST NOT manipulate any resctrl pseudo-filesystems." And in the
likely case where Intel RDT is supported by both the hardware and
kernel but the resctrl filesystem is not mounted, these checks can get
expensive as the intelrdt package needs to parse mountinfo to check
whether the filesystem has been mounted to a non-standard path.
Optimize for the common case of containers with no intelRdt
configuration by only performing the checks when the container has opted
in.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The OCI runtime spec mandates "[i]f intelRdt is not set, the runtime
MUST NOT manipulate any resctrl pseudo-filesystems." Attempting to
delete files counts as manipulating, so stop doing that when the
container's RDT configuration is nil.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The intelrdt package only needs to parse mountinfo to find the mount
point of the resctrl filesystem. Users are generally going to mount the
resctrl filesystem to the pre-created /sys/fs/resctrl directory, so
there is a common case where mountinfo parsing is not required. Optimize
for the common case with a fast path which checks both for the existence
of the /sys/fs/resctrl directory and whether the resctrl filesystem was
mounted to that path using a single statfs syscall.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Reading /proc/cpuinfo is a surprisingly expensive operation. Since
kernel version 4.12 [1], opening /proc/cpuinfo on an x86 system can
block for around 20 milliseconds while the kernel samples the current
CPU frequency. There is a very recent patch [2] which gets rid of the
delay, but has yet to make it into the mainline kenel. Regardless,
kernels for which opening /proc/cpuinfo takes 20ms will continue to be
run in production for years to come. libcontainer only opens
/proc/cpuinfo to read the processor feature flags so all the delays to
get an accurate snapshot of the CPU frequency are just wasted time.
If we wanted to, we could interrogate the CPU features directly from
userspace using the `CPUID` instruction. However, Intel and AMD CPUs
have flags in different positions for their analogous sub-features and
there are CPU quirks [3] which would need to be accounted for. Some
Haswell server CPUs support RDT/CAT but are missing the `CPUID` flags
advertising their support; the kernel checks for support on that
processor family by probing the the hardware using privileged
RDMSR/WRMSR instructions [4]. This sort of probing could not be
implemented in userspace so it would not be possible to check for RDT
feature support in userspace without false negatives on some hardware
configurations.
It looks like libcontainer reads the CPU feature flags as a kind of
optimization so that it can skip checking whether the kernel supports an
RDT sub-feature if the hardware support is missing. As the kernel only
exposes subtrees in the `resctrl` filesystem for RDT sub-features with
hardware and kernel support, checking the CPU feature flags is redundant
from a correctness point of view. Remove the /proc/cpuinfo check as it
is an optimization which actually hurts performance.
[1]: https://unix.stackexchange.com/a/526679
[2]: https://lore.kernel.org/all/20220415161206.875029458@linutronix.de/
[3]: https://github.com/torvalds/linux/blob/7cf6a8a17f5b134b7e783c2d45c53298faef82a7/arch/x86/kernel/cpu/resctrl/core.c#L834-L851
[4]: https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/x86/kernel/cpu/resctrl/core.c#L111-L153
Signed-off-by: Cory Snider <csnider@mirantis.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>
A new version of staticcheck (included into golangci-lint 1.46.2) gives
this new warning:
> libcontainer/factory_linux.go:230:59: SA9008: e refers to the result of a failed type assertion and is a zero value, not the value that was being type-asserted (staticcheck)
> err = fmt.Errorf("panic from initialization: %v, %s", e, debug.Stack())
> ^
> libcontainer/factory_linux.go:226:7: SA9008(related information): this is the variable being read (staticcheck)
> if e, ok := e.(error); ok {
> ^
Apparently, this is indeed a bug. Fix by using a different name for a
new variable, so we can access the old one under "else".
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Due to a bug in commit 9c444070ec, when the user and mount namespaces
are used, and the bind mount is followed by the cgroup mount in the
spec, the cgroup is mounted using the bind mount's mount fd.
This can be reproduced with podman 4.1 (when configured to use runc):
$ podman run --uidmap 0:100:10000 quay.io/libpod/testimage:20210610 mount
Error: /home/kir/git/runc/runc: runc create failed: unable to start container process: error during container init: error mounting "cgroup" to rootfs at "/sys/fs/cgroup": mount /proc/self/fd/11:/sys/fs/cgroup/systemd (via /proc/self/fd/12), flags: 0x20502f: operation not permitted: OCI permission denied
or manually with the spec mounts containing something like this:
{
"destination": "/etc/resolv.conf",
"type": "bind",
"source": "/userdata/resolv.conf",
"options": [
"bind"
]
},
{
"destination": "/sys/fs/cgroup",
"type": "cgroup",
"source": "cgroup",
"options": [
"rprivate",
"nosuid",
"noexec",
"nodev",
"relatime",
"ro"
]
}
The issue was not found earlier since it requires using userns, and even then
mount fd is ignored by mountToRootfs, except for bind mounts, and all the bind
mounts have mountfd set, except for the case of cgroup v1's /sys/fs/cgroup
which is internally transformed into a bunch of bind mounts.
This is a minimal fix for the issue, suitable for backporting.
A test case is added which reproduces the issue without the fix applied.
Fixes: 9c444070ec ("Open bind mount sources from the host userns")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
the struct blockIODevice is used in an exported struct but it is not itself exported rendering that type inaccessible to
outside projects
Signed-off-by: cdoern <cdoern@redhat.com>
systemd emits very loud warnings when the path specified doesn't exist
(which can be the case for some of our default rules). We don't need the
ruleset we give systemd to be completely accurate (we discard some kinds
of wildcard rules anyway) so we can safely skip adding these.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Remove upper bound in integer sanity check
to not restrict the number of socket-activated
sockets passed in.
Closes#3488
Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
Define sizeof(int) as a constant, and also return ENOSYS earlier in the
filter if it doesn't increase the number of instructions we generate
(this is a negligible performance improvement but it does make it easier
to understand the generated filter stub).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
On s390x, syscalls above 255 are multiplexed using the (now otherwise
unused) setup(2) syscall (syscall number 0). If the kernel supports the
syscall then it will correctly translate the syscall number such that
seccomp will correctly detect it -- however, for unknown syscalls the
syscall number remains unchanged. This can be verified by running the
following program under strace:
int main(void)
{
scmp_filter_ctx ctx = seccomp_init(SCMP_ACT_TRAP);
seccomp_load(ctx);
return syscall(439, AT_FDCWD, "asdf", X_OK, 0);
}
Which will then die with the following signal (on pre-5.8 kernels):
--- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP,
si_call_addr=0x3ffb3006c22, si_syscall=__NR_setup,
si_arch=AUDIT_ARCH_S390X} ---
(Note that the si_syscall is __NR_setup, not __NR_faccessat2.)
As a result, the -ENOSYS handling we had previously did not work
completely correctly on s390x because any syscall not supported by the
kernel would be treated as syscall number 0 rather than the actual
syscall number.
Always returning -ENOSYS will not cause any issues because in all of the
cases where this multiplexing occurs, seccomp will see the remapped
syscall number -- and no userspace program will call setup(2)
intentionally (the syscall has not existed in Linux for decades and was
originally a hack used early in Linux init prior to spawning pid1 -- so
you will get -ENOSYS from the kernel anyway).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These are only used from inside the package, and we don't want them to
be public.
The only two methods left are Enable and Disable.
While at it, fix or suppress found lint-extra warnings.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit separates the functionality of setting cgroup device
rules out of libct/cgroups to libct/cgroups/devices package. This
package, if imported, sets the function variables in libct/cgroups and
libct/cgroups/systemd, so that a cgroup manager can use those to manage
devices. If those function variables are nil (when libct/cgroups/devices
are not imported), a cgroup manager returns the ErrDevicesUnsupported
in case any device rules are set in Resources.
It also consolidates the code from libct/cgroups/ebpf and
libct/cgroups/ebpf/devicefilter into libct/cgroups/devices.
Moved some tests in libct/cg/sd that require device management to
libct/sd/devices.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This moves the functionality related to devices, SkipDevices, and
SkipFreezeOnSet to a separate file, in preparation for the next commit.
No code changes.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Do not set inheritable capabilities in runc spec, runc exec --cap,
and in libcontainer integration tests.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Looking through git blame, this was added by commit 9fac18329
aka "Initial commit of runc binary", most probably by mistake.
Obviously, a container should not have access to tun/tap device, unless
it is explicitly specified in configuration.
Now, removing this might create a compatibility issue, but I see no
other choice.
Aside from the obvious misconfiguration, this should also fix the
annoying
> Apr 26 03:46:56 foo.bar systemd[1]: Couldn't stat device /dev/char/10:200: No such file or directory
messages from systemd on every container start, when runc uses systemd
cgroup driver, and the system runs an old (< v240) version of systemd
(the message was presumably eliminated by [1]).
[1] https://github.com/systemd/systemd/pull/10996/commits/d5aecba6e0b7c73657c4cf544ce57289115098e7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The CAP_CHECKPOINT_RESTORE linux capability provides the ability to
update /proc/sys/kernel/ns_last_pid. However, because this file is under
/proc, and by default both K8s and CRI-O specify that /proc/sys should
be mounted as Read-Only, by default even with the capability specified,
a process will not be able to write to ns_last_pid.
To get around this, a pod author can specify a volume mount and a
hostpath to bind-mount /proc/sys/kernel/ns_last_pid. However, runc does
not allow specifying mounts under /proc.
This commit adds /proc/sys/kernel/ns_last_pid to the validProcMounts
string array to enable a pod author to mount ns_last_pid as read-write.
The default remains unchanged; unless explicitly requested as a volume
mount, ns_last_pid will remain read-only regardless of whether or not
CAP_CHECKPOINT_RESTORE is specified.
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
In case statfs("/sys/fs/cgroup/unified") fails with any error other
than ENOENT, current code panics. As IsCgroup2HybridMode is called from
libcontainer/cgroups/fs's init function, this means that any user of
libcontainer may panic during initialization, which is ugly.
Avoid panicking; instead, do not enable hybrid hierarchy support and
report the error (under debug level, not to confuse anyone).
Basically, replace the panic with "turn off hybrid mode support"
(which makes total sense since we were unable to statfs its root).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The only implementation of these is linuxContainer. It does not make
sense to have an interface with a single implementation, and we do not
foresee other types of containers being added to runc.
Remove BaseContainer and Container interfaces, moving their methods
documentation to linuxContainer.
Rename linuxContainer to Container.
Adopt users from using interface to using struct.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since LinuxFactory has become the means to specify containers state
top directory (aka --root), and is only used by two methods (Create
and Load), it is easier to pass root to them directly.
Modify all the users and the docs accordingly.
While at it, fix Create and Load docs (those that were originally moved
from the Factory interface docs).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
StartInitialization does not have to be a method of Factory (while
it is clear why it was done that way initially, now we only have
Linux containers so it does not make sense).
Fix callers and docs accordingly.
No change in functionality.
Also, since this was the only user of libcontainer.New with the empty
string as an argument, the corresponding check can now be removed
from it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The only implementation is LinuxFactory, let's use this directly.
Move the piece of documentation about Create from removed factory.go to
the factory_linux.go.
The LinuxFactory is to be removed later.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Require go 1.17 from now on, since go 1.16 is no longer supported.
Drop go1.16 compatibility.
NOTE we also have to install go 1.18 from Vagrantfile, because
Fedora 35 comes with Go 1.16.x which can't be used.
Note the changes to go.mod and vendor are due to
https://go.dev/doc/go1.17#tools
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Function strings.Title is deprecated as of Go 1.18, because it does not
handle some corner cases good enough. In this case, though, it is
perfectly fine to use it since we have a single ASCII word as an
argument, and strings.Title won't be removed until at least Go 2.0.
Suppress the deprecation warning.
The alternative is to not capitalize the namespace string; this will break
restoring of a container checkpointed by earlier version of runc.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
What used to be godoc.org is now pkg.go.dev, and while the old URLs
still work, they might be broken in the future.
Updated badges are generated via https://pkg.go.dev/badge/
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Fix function docs. In particular, remove the part
which is not true ("verifies that the user isn't trying to set up any
mounts they don't have the rights to do"), and fix the part that
says "that doesn't resolve to root" (which is no longer true since
commit d8b669400a).
2. Replace fmt.Sscanf (which is slow and does lots of allocations)
with strings.TrimPrefix and strconv.Atoi.
3. Add a benchmark for rootlessEUIDMount. Comparing the old and the new
implementations:
name old time/op new time/op delta
RootlessEUIDMount-4 1.01µs ± 2% 0.16µs ± 1% -84.15% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
RootlessEUIDMount-4 224B ± 0% 80B ± 0% -64.29% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
RootlessEUIDMount-4 7.00 ± 0% 1.00 ± 0% -85.71% (p=0.008 n=5+5)
Note this code is already tested (in rootless_test.go).
Fixes: d8b669400a
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin