Remove unused code and argument from the ActivationFiles,
and simplify its usage.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 6ede591761)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It appears that when we import github.com/coreos/go-systemd/activation,
it brings in the whole crypto/tls package (which is not used by runc
directly or indirectly), making the runc binary size larger and
potentially creating issues with FIPS compliance.
Let's copy the code of function we use from go-systemd/activation
to avoid that.
The space savings are:
$ size runc.before runc.after
text data bss dec hex filename
7101084 5049593 271560 12422237 bd8c5d runc.before
6508796 4623281 229128 11361205 ad5bb5 runc.after
Reported-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit ba9e60f7a8)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The modernize documentation used to suggest -test flag but it's not
needed as it is enabled by default. Drop it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit dbc4234607)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since we use modernize@latest, it may require latest Go as well (and now it does),
so use "go-version: stable" explicitly (which resolves to latest Go).
This fixes the issue with CI:
> go: golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest: golang.org/x/tools/gopls@v0.21.0 requires go >= 1.25 (running go 1.24.11; GOTOOLCHAIN=local)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 16ee2bbf4c)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case early stage of runc init (nsenter) fails for some reason, it
logs error(s) with FATAL log level, via bail().
The runc init log is read by a parent (runc create/run/exec) and is
logged via normal logrus mechanism, which is all fine and dandy, except
when `runc init` fails, we return the error from the parent (which is
usually not too helpful, for example):
runc run failed: unable to start container process: can't get final child's PID from pipe: EOF
Now, the actual underlying error is from runc init and it was logged
earlier; here's how full runc output looks like:
FATA[0000] nsexec-1[3247792]: failed to unshare remaining namespaces: No space left on device
FATA[0000] nsexec-0[3247790]: failed to sync with stage-1: next state
ERRO[0000] runc run failed: unable to start container process: can't get final child's PID from pipe: EOF
The problem is, upper level runtimes tend to ignore everything except
the last line from runc, and thus error reported by e.g. docker is not
very helpful.
This patch tries to improve the situation by collecting FATAL errors
from runc init and appending those to the error returned (instead of
logging). With it, the above error will look like this:
ERRO[0000] runc run failed: unable to start container process: can't get final child's PID from pipe: EOF; runc init error(s): nsexec-1[141549]: failed to unshare remaining namespaces: No space left on device; nsexec-0[141547]: failed to sync with stage-1: next state
Yes, it is long and ugly, but at least the upper level runtime will
report it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit f944ccecb2)
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>
(cherry picked from commit 6c18b25cdc)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.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>
(cherry picked from commit aea52d0ab0)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.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>
(cherry picked from commit 067b8335e7)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.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>
(cherry picked from commit 9c8f476cb6)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Since the recent CVE fixes, TestFdLeaksSystemd sometimes fails:
=== RUN TestFdLeaksSystemd
exec_test.go:1750: extra fd 9 -> /12224/task/13831/fd
exec_test.go:1753: found 1 extra fds after container.Run
--- FAIL: TestFdLeaksSystemd (0.10s)
It might have been caused by the change to the test code in commit
ff6fe13 ("utils: use safe procfs for /proc/self/fd loop code") -- we are
now opening a file descriptor during the logic to get a list of file
descriptors. If the file descriptor happens to be allocated to a
different number, you'll get an error.
Let's try to filter out the fd used to read a directory.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 5fbc3bb019)
Signed-off-by: lifubang <lifubang@acmcoder.com>
We intentionally broke this in commit d40b3439a9 ("rootfs: switch to
fd-based handling of mountpoint targets") under the assumption that most
users do not need this feature. Sadly it turns out they do, and so
commit 3f925525b4 ("rootfs: re-allow dangling symlinks in mount
targets") added a hotfix to re-add this functionality.
This patch adds some much-needed tests for this behaviour, since it
seems we are going to need to keep this for compatibility reasons (at
least until runc v2...).
Co-developed-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 15d7c214cd)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
While CreateInRoot supports hallucinating the target path, we do not use
it directly when constructing device inode targets because we need to
have different handling for mknod and bind-mounts.
The solution is to simply have a more generic MkdirAllParentInRoot
helper that MkdirAll's the parent directory of the target path and then
allows the caller to create the trailing component however they like.
(This can be used by CreateInRoot internally as well!)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 195e9551e4)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
In order to maintain compatibility with previous releases of runc (which
permitted dangling symlinks as path components by permitting
non-existent path components to be treated like real directories) we
have to first do SecureJoin to construct a target path that is
compatible with the old behaviour but has all dangling symlinks (or
other invalid paths like ".." components after non-existent directories)
removed.
This is effectively a more generic verison of commit 3f925525b4
("rootfs: re-allow dangling symlinks in mount targets") and will let us
remove the need for open-coding SecureJoin workarounds.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit cfb74326be)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Now that MkdirAllInRoot has been removed, we can make MkdirAllInRootOpen
less wordy by renaming it to MkdirAllInRoot. This is a non-functional
change.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 20c5a8ec4a)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This probably should've been done as part of commit d40b3439a9
("rootfs: switch to fd-based handling of mountpoint targets") but it
seems I missed them when doing the rest of the conversions.
This also lets us remove utils.WithProcfd entirely, as well as
pathrs.MkdirAllInRoot. Unfortunately, WithProcfd was exposed in the
externally-importable "libcontainer/utils" package and so we need to
have a deprecation notice to remove it in runc 1.5.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 9dbd37e06f)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These helpers will be needed for the compatibility code added in future
patches in this series, but because "internal/pathrs" is imported by
"libcontainer/utils" we need to move them so that we can avoid circular
dependencies.
Because the old functions were in a non-internal package it is possible
some downstreams use them, so add some wrappers but mark them as
deprecated.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 42a1e19d67)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This is to ensure that Go version in Dockerfile (which is used to build
release binaries) is:
- currently supported;
- used in CI tests.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit df4acc8867)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For some reason, some jobs in .github/workflows/validate.yml
have "fetch-depth: 0" argument to actions/checkout, meaning
"all history for all branches and tags". Obviously this is
not needed here.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit e0b00171eb0f338cf024760019abdd4e7dec690f)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
On some systems (e.g., AlmaLinux 8), systemd automatically removes cgroup paths
when they become empty (i.e., contain no processes). To prevent this, we spawn
a dummy process to pin the cgroup in place.
Fix: https://github.com/opencontainers/runc/issues/5003
Signed-off-by: lifubang <lifubang@acmcoder.com>
(cherry picked from commit bba7647d09)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This is much easier to read and removes the need for explicit per-type
helper functions.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 8ab2458bc4)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The main update is actually in github.com/opencontainers/cgroups, but we
need to also update runtime-spec to a newer pre-release version to get
the updates from there as well.
In short, the behaviour change is now that "0" is treated as a valid
value to set in "pids.max", "-1" means "max" and unset/nil means "do
nothing". As described in the opencontainers/cgroups PR, this change is
actually backwards compatible because our internal state.json stores
PidsLimit, and that entry is marked as "omitempty". So, an old runc
would omit PidsLimit=0 in state.json, and this will be parsed by a new
runc as being "nil" -- and both would treat this case as "do not set
anything".
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 3b75374cc7)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
dependabot[bot]