Example code in README is outdated (especially since cgroups is moved to
a separate repository) and lacks proper import statements. And, since it
is not code, it is hard to keep it up to date.
Let's move it out to the example_test.go file and refer to it. Note we
still don't run it, but it will be compiled and linted in CI.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
SCMP_ACT_KILL terminates the process with a fatal signal, which may
produce a core dump depending on the host configuration.
While this is harmless on ephemeral CI instances, it can leave unwanted
core files on developer or customer systems. It also interferes with
test environments that detect unexpected core dumps.
Signed-off-by: Ricardo Branco <rbranco@suse.de>
When container-selinux 4:2.246.0-1.el10 is installed, it produces the
following %post script warnings:
> ...
> Running scriptlet: container-selinux-4:2.246.0-1.el10.noarch 26/37
> Installing : container-selinux-4:2.246.0-1.el10.noarch 26/37
> Running scriptlet: container-selinux-4:2.246.0-1.el10.noarch 26/37
> libsemanage.semanage_pipe_data: Child process /usr/libexec/selinux/hll/pp failed with code: 255. (No data available).
> libsemanage.semanage_compile_module: container: libsepol.policydb_read: policydb module version 24 does not match my version range 4-23.
> libsemanage.semanage_compile_module: container: libsepol.sepol_module_package_read: invalid module in module package (at section 0).
> libsemanage.semanage_compile_module: container: libsepol.sepol_ppfile_to_module_package: Failed to read policy package.
> libsemanage.semanage_direct_commit: Failed to compile hll files into cil files. (No data available).
> semodule: Failed!
> ...
For some reason, dnf install still succeeds, but when the selinux tests
fail with:
> chcon: failed to change context of '/tmp/bats-run-3MMyYP/runc.szTqBc/bundle/runc' to ‘system_u:object_r:container_runtime_exec_t:s0’: Invalid argument
All this is fixed once policycoreutils is added to the list of RPMS so
it is updated (from 3.9-3.el10 to 3.10-1.el10) during the same
transaction.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 67f6c37b ("ci/gha: switch to ubuntu 24.04") switched most GHA CI
to Ubuntu 24.04 except for one job. It says:
> Leave ubuntu-22.04 for ci/cross-i386 (issue with systemctl restart hang
> after apt install). This can be addressed separately later.
Assuming the issue it already fixed (updated systemd or something),
let's finalize the 24.04 switch.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Previously, when prepareCgroupFD would not open container's cgroup
(as configured in config.json and saved to state.json), it returned
a fatal error, as we presumed a container can't exist without its own
cgroup.
Apparently, it can. In a case when container is configured without
cgroupns (i.e. it uses hosts cgroups), and /sys/fs/cgroup is mounted
read-write, a rootful container's init can move itself to an entirely
different cgroup (even a new one that it just created), and then the
original container cgroup is removed by the kernel (or systemd?) as
it has no processes left. By the way, from the systemd point of view
the container is gone. And yet it is still there, and users want
runc exec to work!
And it worked, thanks to the "let's try container init's cgroup"
fallback as added by commit c91fe9aeba ("cgroup2: exec: join the
cgroup of the init process on EBUSY"). The fallback was added for
the entirely different reason, but it happened to work in this very
case, too.
This behavior was broken with the introduction of CLONE_INTO_CGROUP
support.
While it is debatable whether this is a valid scenario when a container
moves itself into a different cgroup, this very setup is used by e.g.
buildkitd running in a privileged kubernetes container (see issue 5089).
To restore the way things are expected to work, add the same "try
container init's cgroup" fallback into prepareCgroupFD.
While at it, simplify the code flow.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Refactor addIntoCgroupV2 in an attempt to simplify it.
2. Fix the bug of not trying the init cgroup fallback if
rootlessCgroup is set. This is a bug because rootlessCgroup
tells to ignore cgroup join errors, not to never try the fallback.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Separate initProcessCgroupPath code out of addIntoCgroupV2.
To be used by the next patch.
While at it, describe the new scenario in which the container's
configured cgroup might not be available.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When parsing mount options into recAttrSet and recAttrClr,
the code sets attr_clr to individual atime flags (e.g.
MOUNT_ATTR_NOATIME or MOUNT_ATTR_STRICTATIME) when clearing
atime attributes. However, this violates the kernel's
requirement documented in mount_setattr(2)[1]:
> Note that, since the access-time values are an enumeration
> rather than bit values, a caller wanting to transition to a
> different access-time setting cannot simply specify the
> access-time setting in attr_set, but must also include
> MOUNT_ATTR__ATIME in the attr_clr field. The kernel will
> verify that MOUNT_ATTR__ATIME isn't partially set in
> attr_clr (i.e., either all bits in the MOUNT_ATTR__ATIME
> bit field are either set or clear), and that attr_set
> doesn't have any access-time bits set if MOUNT_ATTR__ATIME
> isn't set in attr_clr.
Passing only a single atime flag (e.g. MOUNT_ATTR_RELATIME) in
attr_clr causes mount_setattr() to fail with EINVAL.
This change ensures that whenever an atime mode is updated,
attr_clr includes MOUNT_ATTR__ATIME to properly reset the
entire access-time attribute field before applying the new mode.
[1] https://man7.org/linux/man-pages/man2/mount_setattr.2.html
Signed-off-by: lifubang <lifubang@acmcoder.com>
Since [PR 4812], runc exec tries to use clone3 syscall with
CLONE_INTO_CGROUP, falling back to the old method if it is not
supported.
One issue with that approach is, a
> Cmd cannot be reused after calling its [Cmd.Start], [Cmd.Run],
> [Cmd.Output], or [Cmd.CombinedOutput] methods.
(from https://pkg.go.dev/os/exec#Cmd).
This is enforced since Go 1.26, see [CL 728642], and so runc exec
actually fails in specific scenarios (go1.26 and no CLONE_INTO_CGROUP
support).
The easiest workaround is to pre-copy the p.cmd structure (copy = *cmd).
From the [CL 734200] it looks like it is an acceptable way, but it might
break in the future as it also copies the private fields, so let's do a
proper field-by-field copy. If the upstream will add cmd.Clone method,
we will switch to it.
Also, we can probably be fine with a post-copy (once the first Start has
failed), but let's be conservative here and do a pre-copy.
[PR 4812]: https://github.com/opencontainers/runc/pull/4812
[CL 728642]: https://go.dev/cl/728642
[CL 734200]: https://go.dev/cl/734200
Reported-by: Efim Verzakov <efimverzakov@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Theoretically, exec.Command can set cmd.Err.
Practically, this should never happen (Linux, Go <= 1.26, exePath is
absolute), but in the unlikely case it does, let's fail early.
This is related to the cloneCmd (to be introduced by the following
commit) which chooses to not copy the Err field. Theoretically,
exec.Command can set Err and so the first call to cmd.Start will fail
(since Err != nil), and the second call to cmd.Start may succeed because
Err == nil. Yet, this scenario is highly unlikely, but better be safe
than sorry.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The Process type is quite big (currently 368 bytes on a 64 bit Linux)
and using non-pointer receivers in its methods results in copying which
is totally unnecessary.
Change the methods to use pointer receivers.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The Config type is quite big (currently 554 bytes on a 64 bit Linux)
and using non-pointer receivers in its methods results in copying which
is totally unnecessary.
Change the methods to use pointer receivers.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Rename a function parameter (containerId -> containerID) to avoid a
linter warning:
> var-naming: method parameter containerId should be containerID (revive)
In many other places, including config.json (.linux.uidMappings and
.gidMappings) it is already called containerID, so let's rename.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Apparently Write (and WriteString) must return an error (apparently
io.ErrShortWrite) on short writes (see [1], [2]), so no explicit check
for a short write is needed.
While at it, use (*os.File).WriteString directly rather than
io.WriteString.
[1]: https://pkg.go.dev/os#File.Write
[2]: https://pkg.go.dev/io#Writer
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>