filepath-securejoin v0.3 gave us a much safer re-open primitive, we
should use it to avoid any theoretical attacks. Rather than using it
direcly, add a small pathrs wrapper to make libpathrs migrations in the
future easier...
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
We will have more wrappers around filepath-securejoin, and so move them
to their own specific package so that we can eventually use libpathrs
fairly cleanly (by swapping out the implementation).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Introduce and use iobail, xread, and xwrite wrappers so that we can
properly check read/write return value and call either bail or bailx on
error, with proper diagnostics (distinguishing failed read/write from a
short read/write).
This prevents the "Success" prefix in errors like:
failed to sync with stage-1: next state: Success
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a few missing sane_kill calls where they make sense.
Remove one useless sane_kill of stage2_pid, as during SYNC_USERMAP stage2
is not yet started. It is harmless yet it makes the code slightly harder
to read.
Set the child pid to -1 upon receiving SYNC_CHILD_FINISH
to minimize the chances of killing an unrelated process.
When a child sends SYNC_CHILD_FINISH it is about to exit
(although theoretically it could be stuck during debug logging).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We use bail to report fatal errors, and bail always append %m
(aka strerror(errno)). In case an error condition did not set
errno, the log message will end up with ": Success" or an error
from a stale errno value. Either case is confusing for users.
Introduce bailx which is the same as bail except it does not
append %m, and use it where appropriate.
The naming follows libc's err(3) and errx(3).
PS we still use bail in a few cases after read or write, even
if that read/write did not return an error, because the code
does not distinguish between short read/write and error (-1).
This will be addressed by the next commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since sane_kill after a failed read or write, but before reporting the
error from that read or write, it may change the errno value in case
kill(2) fails.
Save and restore the errno around the call to kill.
While at it,
- change the code to return early;
- don't return kill return value as no one is using it, and the errno
value no longer correlates.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The commit mentioned below has missed these changes.
Fixes: 17570625 ("Use for range over integers")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
Factor startWithCgroupFD out of start to reduce the start complexity.
This also implements a more future-proof way of calling p.comm.closeChild.
Co-authored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The (*setns).start is supposed to close child fds once the child has
started, or upon an error. Commit 5af4dd4e6 added a bug -- child fds
are not closed if prepareCgroupFD fails.
Fix by adding a missing call to closeChild.
I'm not sure how to write a good test case for it. Found when working
on PR 4928 (and tested in there).
Fixes: 5af4dd4e6
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
strings.Title is deprecated since Go 1.18. Replace it with a simple
manual capitalization of the first character in criuNsToKey().
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Now that we've updated to golang.org/x/sys@v0.37.0, CPUSet has a Fill
helper that does the equivalent to our underflow trick to make setting
all CPUs efficient.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This package was marked deprecated in commit 9b60a93cf3
("libcontainer/userns: migrate to github.com/moby/sys/userns"), which
was included in runc 1.2. Users have thus had a year to migrate to
github.com/moby/sys/userns and it's okay for us to remove this wrapper
package.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
ba0b5e26 ("libcontainer: remove all mount logic from nsexec") introduced
a request function that handles two tasks:
- the exchanges with the request and response channels
- the closing of the request channel.
From 1.21, the closing of the request channel may be done with
context.AfterFunc(). Moreover, context.AfterFunc() is guaranteed to run
once.
Link: https://pkg.go.dev/context#AfterFunc
Suggested-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
Set the process personality early to ensure it takes effect before
seccomp is initialized. If seccomp filters are applied first and they
block personality-related system calls (e.g., `personality(2)`),
subsequent attempts to set the personality will fail.
Signed-off-by: lifubang <lifubang@acmcoder.com>
The linux.intelRdt.enableMonitoring field enables the creation of
a per-container monitoring group. The monitoring group is removed when
the container is destroyed.
Signed-off-by: Markus Lehtonen <markus.lehtonen@intel.com>
The main benefit here is when we are using a systemd cgroup driver,
we actually ask systemd to add a PID, rather than doing it ourselves.
This way, we can add rootless exec PID to a cgroup.
This requires newer opencontainers/cgroups and coreos/go-systemd.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Remove cgroupPaths field from struct setnsProcess, because:
- we can get base cgroup paths from p.manager.GetPaths();
- we can get sub-cgroup paths from p.process.SubCgroupPaths.
But mostly because we are going to need separate cgroup paths when
adopting cgroups.AddPid.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Implement support for the linux.intelRdt.schemata field of the spec.
This allows management of the "schemata" file in the resctrl group in a
generic way.
Signed-off-by: Markus Lehtonen <markus.lehtonen@intel.com>
These sysctls are all per-userns (termed `ucounts` in the kernel code) are
settable with CAP_SYS_RESOURCE in the user namespace.
Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
libcontainer/user was marked as deprecated in d9ea71bf96 ("deprecate
libcontainer/user") and users have had plenty of time to migrate to
github.com/moby/sys/user.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
In certain deployments, it's possible for runc to be spawned by a
process with a restrictive cpumask (such as from a systemd unit with
CPUAffinity=... configured) which will be inherited by runc and thus the
container process by default.
The cpuset cgroup used to reconfigure the cpumask automatically for
joining processes, but kcommit da019032819a ("sched: Enforce user
requested affinity") changed this behaviour in Linux 6.2.
The solution is to try to emulate the expected behaviour by resetting
our cpumask to correspond with the configured cpuset (in the case of
"runc exec", if the user did not configure an alternative one). Normally
we would have to parse /proc/stat and /sys/fs/cgroup, but luckily
sched_setaffinity(2) will transparently convert an all-set cpumask (even
if it has more entries than the number of CPUs on the system) to the
correct value for our usecase.
For some reason, in our CI it seems that rootless --systemd-cgroup
results in the cpuset (presumably temporarily?) being configured such
that sched_setaffinity(2) will allow the full set of CPUs. For this
particular case, all we care about is that it is different to the
original set, so include some special-casing (but we should probably
investigate this further...).
Reported-by: ningmingxiao <ning.mingxiao@zte.com.cn>
Reported-by: Martin Sivak <msivak@redhat.com>
Reported-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Aleksa Sarai