Commit Graph

2397 Commits

Author SHA1 Message Date
Aleksa Sarai e65d4cac66 specconv: temporarily allow userns path and mapping if they match
(This is a cherry-pick of ebcef3e651e61aeee96546301d8db9e92b505ce6.)

It turns out that the error added in commit 09822c3da8 ("configs:
disallow ambiguous userns and timens configurations") causes issues with
containerd and CRIO because they pass both userns mappings and a userns
path.

These configurations are broken, but to avoid the regression in this one
case, output a warning to tell the user that the configuration is
incorrect but we will continue to use it if and only if the configured
mappings are identical to the mappings of the provided namespace.

Fixes: 09822c3da8 ("configs: disallow ambiguous userns and timens configurations")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-14 12:17:17 +11:00
Aleksa Sarai 8f8cb45572 configs: disallow ambiguous userns and timens configurations
(This is a cherry-pick of 09822c3da8ad8fa91b8796c5abf27ef06814a0c3.)

For userns and timens, the mappings (and offsets, respectively) cannot
be changed after the namespace is first configured. Thus, configuring a
container with a namespace path to join means that you cannot also
provide configuration for said namespace. Previously we would silently
ignore the configuration (and just join the provided path), but we
really should be returning an error (especially when you consider that
the configuration userns mappings are used quite a bit in runc with the
assumption that they are the correct mapping for the userns -- but in
this case they are not).

In the case of userns, the mappings are also required if you _do not_
specify a path, while in the case of the time namespace you can have a
container with a timens but no mappings specified.

It should be noted that the case checking that the user has not
specified a userns path and a userns mapping needs to be handled in
specconv (as opposed to the configuration validator) because with this
patchset we now cache the mappings of path-based userns configurations
and thus the validator can't be sure whether the mapping is a cached
mapping or a user-specified one. So we do the validation in specconv,
and thus the test for this needs to be an integration test.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-14 11:56:43 +11:00
Aleksa Sarai 0c8e2cc602 *: actually support joining a userns with a new container
(This is a cherry-pick of 1912d5988bbb379189ea9ceb2e03945738c513dc.)

Our handling for name space paths with user namespaces has been broken
for a long time. In particular, the need to parse /proc/self/*id_map in
quite a few places meant that we would treat userns configurations that
had a namespace path as if they were a userns configuration without
mappings, resulting in errors.

The primary issue was down to the id translation helper functions, which
could only handle configurations that had explicit mappings. Obviously,
when joining a user namespace we need to map the ids but figuring out
the correct mapping is non-trivial in comparison.

In order to get the mapping, you need to read /proc/<pid>/*id_map of a
process inside the userns -- while most userns paths will be of the form
/proc/<pid>/ns/user (and we have a fast-path for this case), this is not
guaranteed and thus it is necessary to spawn a process inside the
container and read its /proc/<pid>/*id_map files in the general case.

As Go does not allow us spawn a subprocess into a target userns,
we have to use CGo to fork a sub-process which does the setns(2). To be
honest, this is a little dodgy in regards to POSIX signal-safety(7) but
since we do no allocations and we are executing in the forked context
from a Go program (not a C program), it should be okay. The other
alternative would be to do an expensive re-exec (a-la nsexec which would
make several other bits of runc more complicated), or to use nsenter(1)
which might not exist on the system and is less than ideal.

Because we need to logically remap users quite a few times in runc
(including in "runc init", where joining the namespace is not feasable),
we cache the mapping inside the libcontainer config struct. A future
patch will make sure that we stop allow invalid user configurations
where a mapping is specified as well as a userns path to join.

Finally, add an integration test to make sure we don't regress this again.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2023-12-14 11:56:33 +11:00
Heran Yang 87792ce02c libct/cg: add swapOnlyUsage in MemoryStats
This field reports swap-only usage. For cgroupv1, `Usage` and `Failcnt`
are set by subtracting memory usage from memory+swap usage. For cgroupv2,
`Usage`, `Limit`, and `MaxUsage` are set. This commit also export `MaxUsage`
of memory under cgroupv2 mode, using `memory.peak` introduced in kernel 5.19.

Signed-off-by: Heran Yang <heran55@126.com>
(cherry picked from commit 104b8dc951)
Signed-off-by: Harshal Patil <harpatil@redhat.com>
2023-12-12 11:42:09 -05:00
Kir Kolyshkin b426e9b76b libct/cgroups.OpenFile: clean "file" argument
This prevents potential exploit of using "../" in cgroups.OpenFile
(as well as other methods that use OpenFile) to read or write to
other cgroups.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 2c9598c886)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-10-27 10:47:20 -07:00
Kir Kolyshkin 8214e63474 libct/cg: support hugetlb rsvd
This adds support for hugetlb.<pagesize>.rsvd limiting and accounting.

The previous non-rsvd max/limit_in_bytes does not account for reserved
huge page memory, making it possible for a processes to reserve all the
huge page memory, without being able to allocate it (due to cgroup
restrictions).

In practice this makes it possible to successfully mmap more huge page
memory than allowed via the cgroup settings, but when using the memory
the process will get a SIGBUS and crash. This is bad for applications
trying to mmap at startup (and it succeeds), but the program crashes
when starting to use the memory. eg. postgres is doing this by default.

This also keeps writing to the old max/limit_in_bytes, for backward
compatibility.

More info can be found here: https://lkml.org/lkml/2020/2/3/1153

(commit message mostly written by Odin Ugedal)

[1.1 backport: check for CGROUP_UNIFIED in integration test]

Co-authored-by: Odin Ugedal <odin@ugedal.com>
(cherry picked from commit 4a7d3ae5cd)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-10-20 11:44:36 -07:00
lifubang b365458f40 fix a typo in cloned_binary.c: re-use -> reuse
Signed-off-by: lifubang <lifubang@acmcoder.com>
2023-10-04 13:24:36 +08:00
lifubang 8f66c9fbd0 fix two typos
Signed-off-by: lifubang <lifubang@acmcoder.com>
(cherry picked from commit 109dcadd9d)
Signed-off-by: lifubang <lifubang@acmcoder.com>
2023-10-04 13:17:41 +08:00
Jordan Rife 016b2b4233 Handle kmem.limit_in_bytes removal
kmem.limit_in_bytes has been removed in upstream linux and this patch
is queued to be backported to linux 6.1 stable:

- https://lore.kernel.org/linux-mm/20230705134434.GA156754@cmpxchg.org/T/
- https://www.spinics.net/lists/stable-commits/msg316619.html

Without this change to libcontainerd, GetStats() will return an error
on the latest kernel(s). A downstream effect is that Kubernetes's
kubelet does not start up. This fix was tested by ensuring that it
unblocks kubelet startup when running on the latest kernel.

Signed-off-by: Jordan Rife <jrife0@gmail.com>
(cherry picked from commit 99469eba3e)
Signed-off-by: Jordan Rife <jrife0@gmail.com>
2023-09-26 16:16:16 +00:00
Cory Snider f44190e076 libct/intelrdt: check if available iff configured
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>
(cherry picked from commit ea0bd78268)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Cory Snider 6cf9ac159e libct/intelrdt: skip remove unless configured
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>
(cherry picked from commit 56daf36be2)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Cory Snider 4796f49ca3 libct/intelrdt: elide parsing mountinfo
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>
(cherry picked from commit c156bde7cc)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Cory Snider 6a7a6a5741 libct/intelrdt: skip reading /proc/cpuinfo
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>
(cherry picked from commit 9f107489b0)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Cory Snider 7c83dbe6ec libct/intelrdt: delete IsMBAScEnabled()
This function is unused, and removing it simplifies the changes which
follow this commit.

Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 13674f43d3)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Kir Kolyshkin 5ebcfa6281 [1.1] libct: rm intelrtd.Manager interface, NewIntelRdtManager
[This is a manual port of commit dbd990d555 to release-1.1
branch. Original description follows.]

Remove intelrtd.Manager interface, since we only have a single
implementation, and do not expect another one.

Rename intelRdtManager to Manager, and modify its users accordingly.

Remove NewIntelRdtManager from factory.

Remove IntelRdtfs. Instead, make intelrdt.NewManager return nil if the
feature is not available.

Remove TestFactoryNewIntelRdt as it is now identical to TestFactoryNew.

Add internal function newManager to be used for tests (to make sure
some testing is done even when the feature is not available in
kernel/hardware).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Kir Kolyshkin 69473d0af9 libct: rm TestGetContainerStats, mockIntelRdtManager
TestGetContainerStats test a function that is smaller than the test
itself, and only calls a couple of other functions (which are
represented by mocks). It does not make sense to have it.

mockIntelRdtManager is only needed for TestGetContainerStats
and TestGetContainerState, which basically tests that Path
is called. Also, it does not make much sense to have it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 85932850ec)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Kir Kolyshkin dfdc7d078f libct/intelrdt: explain why mountinfo is required
For the Nth time I wanted to replace parsing mountinfo with
statfs and the check for superblock magic, but it is not possible
since some code relies of mount options check which can only
be obtained via mountinfo.

Add a note about it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 5e201e7ce2)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Kir Kolyshkin 5ba1b8ec43 libct/intelrdt: faster init if rdt is unsupported
In a (quite common) case RDT is not supported by the kernel/hardware,
it does not make sense to parse /proc/cpuinfo and /proc/self/mountinfo,
and yet the current code does it (on every runc exec, for example).

Fortunately, there is a quick way to check whether RDT is available --
if so, kernel creates /sys/fs/resctrl directory. Check its existence,
and skip all the other initialization if it's not present.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit edeb3b376c)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Kir Kolyshkin a5407b9a36 libct/intelrdt: remove findMountpointDir test
This test was written back in the day when findIntelRdtMountpointDir
was using its own mountinfo parser. Commit f1c1fdf911 changed that,
and thus this test is actually testing moby/sys/mountinfo parser, which
does not make much sense.

Remove the test, and drop the io.Reader argument since we no longer need
to parse a custom file.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 6c6b14e075)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Kir Kolyshkin dc8d0cc172 libct/intelrdt: wrap Root in sync.Once
In case resctrl filesystem can not be found in /proc/self/mountinfo
(which is pretty common on non-server or non-x86 hardware), subsequent
calls to Root() will result in parsing it again and again.

Use sync.Once to avoid it. Make unit tests call it so that Root() won't
actually parse mountinfo in tests.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 02e961bcf9)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-10 17:09:53 +08:00
Alexander Eldeib 929d04fc8b libct/cg/fs2: use file + anon + swap for usage
This aligns v2 usage calculations more closely with v1.
Current node-level reporting for v1 vs v2 on the same
machine under similar load may differ by ~250-750Mi.

Also return usage as combined swap + memory usage, aligned
with v1 and non-root v2 cgroups.

`mem_cgroup_usage` in the kernel counts NR_FILE_PAGES
+ NR_ANON_MAPPED + `nr_swap_pages` (if swap enabled) [^0].

Using total - free results in higher "usage" numbers.
This is likely due to various types of reclaimable
memory technically counted as in use (e.g. inactive anon).

See also https://github.com/kubernetes/kubernetes/issues/118916 for more context

[^0]: https://github.com/torvalds/linux/blob/06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5/mm/memcontrol.c#L3673-L3680

Signed-off-by: Alexander Eldeib <alexeldeib@gmail.com>
2023-08-09 19:20:15 -07:00
Kir Kolyshkin bdbfe0425f ci: bump golangci-lint, remove fixed exception
The exception was fixed by https://github.com/polyfloyd/go-errorlint/pull/12
which eventually made its way into golangci-lint.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 98317c16ed)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-09 16:44:50 -07:00
Kir Kolyshkin 44a53f088c ci: fix TestOpenat2 when no systemd is used
A few cases relied on the fact that systemd is used, and thus
/sys/fs/cgroup/user.slice is available.

Guess what, in case of "make unittest" it might not be.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 5c6b334c88)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-08 18:47:03 -07:00
Kir Kolyshkin cff41a893c ci: fix TestNilResources when systemd not available
Split the test into two -- for fs and systemd cgroup managers, and only
run the second one if systemd is available.

Prevents the following failure during `make unittest`:

> === RUN   TestNilResources
>     manager_test.go:27: systemd not running on this host, cannot use systemd cgroups manager
> --- FAIL: TestNilResources (0.22s)

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 962019d64e)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-08-08 18:46:40 -07:00
lifubang 3d3a2b38b4 fix some file mode bits missing when doing mount syscall
Signed-off-by: lifubang <lifubang@acmcoder.com>
(cherry picked from commit 6092a4b42d)
2023-08-03 11:31:03 +08:00
lfbzhm 096277c761 Merge pull request #3921 from jiusanzhou/1.1-bugfix/skip-update-while-frozen-faield
[1.1] libct/cg/sd/v1: do not update non-frozen cgroup after frozen failed.
2023-07-16 08:16:43 +08:00
Kir Kolyshkin 1188c5a192 runc delete: call systemd's reset-failed
runc delete is supposed to remove all the container's artefacts.
In case systemd cgroup driver is used, and the systemd unit has failed
(e.g. oom-killed), systemd won't remove the unit (that is, unless the
"CollectMode: inactive-or-failed" property is set).

Call reset-failed from manager.Destroy so the failed unit will be
removed during "runc delete".

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 43564a7b55)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-07-08 11:54:41 -07:00
Kir Kolyshkin 71e7600768 libct/cg/sd: remove logging from resetFailedUnit
Sometimes we call resetFailedUnit as a cleanup measure, and we don't
care if it fails or not. So, move error reporting to its callers, and
ignore error in cases we don't really expect it to succeed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 91b4cd25b7)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-07-08 11:54:20 -07:00
Zoe 6bc3f22a31 libct/cg/sd/v1: do not update non-frozen cgroup after frozen failed.
In code we have frozen the cgroup to avoid the processes get
an occasional "permission denied" error, while the systemd's application of device
rules is done disruptively. When the processes in the container can not
be frozen over 2 seconds (which defined in fs/freezer.go),
we still update the cgroup which resulting the container get an occasional
"permission denied" error in some cases.

Return error directly without updating cgroup, when freeze fails.

Fixes: #3803

Signed-off-by: Zoe <hi@zoe.im>
2023-06-30 19:23:36 +08:00
Akihiro Suda 9164fe17a7 libct/seccomp: add riscv64
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 1d7b297128)
2023-06-28 22:34:45 +03:00
Brian Goff 9af462e4a7 Fix tmpfs mode opts when dir already exists
When a directory already exists (or after a container is restarted) the
perms of the directory being mounted to were being used even when a
different permission is set on the tmpfs mount options.

This prepends the original directory perms to the mount options.
If the perms were already set in the mount opts then those perms will
win.
This eliminates the need to perform a chmod after mount entirely.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 9fa8b9de3e)
Resolved conflicts:
	tests/integration/run.bats
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
2023-06-28 08:21:04 -06:00
Sebastiaan van Stijn bcf234b897 Merge pull request #3877 from kolyshkin/1.1-sd-rm-race
[1.1] libct: fix a race with systemd removal
2023-05-30 14:34:20 +02:00
Aleksa Sarai 63af8b0069 init: do not print environment variable value
When given an environment variable that is invalid, it's not a good idea
to output the contents in case they are supposed to be private (though
such a container wouldn't start anyway so it seems unlikely there's a
real way to use this to exfiltrate environment variables you didn't
already know).

Reported-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 20e38fb2b1)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-05-22 14:52:57 -07:00
Kir Kolyshkin 404ea7ab0f libct: fix a race with systemd removal
For a previous attempt to fix that (and added test cases), see commit
9087f2e827.

Alas, it's not always working because of cgroup directory TOCTOU.

To solve this and avoid the race, add an error _after_ the operation.
Implement it as a method that ignores the error that should be ignored.
Instead of currentStatus(), use faster runType(), since we are not
interested in Paused status here.

For Processes(), remove the pre-op check, and only use it after getting
an error, making the non-error path more straightforward.

For Signal(), add a second check after getting an error. The first check
is left as is because signalAllProcesses might print a warning if the
cgroup does not exist, and we'd like to avoid that.

This should fix an occasional failure like this one:

	not ok 84 kill detached busybox
	# (in test file tests/integration/kill.bats, line 27)
	#   `[ "$status" -eq 0 ]' failed
	....
	# runc kill test_busybox KILL (status=0):
	# runc kill -a test_busybox 0 (status=1):
	# time="2023-04-04T18:24:27Z" level=error msg="lstat /sys/fs/cgroup/devices/system.slice/runc-test_busybox.scope: no such file or directory"

(cherry picked from commit fe278b9caa)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-05-22 14:06:04 -07:00
Kir Kolyshkin 79a52b4349 libct/cg/sd: use systemd version when generating dev props
Commit 343951a22b added a call to os.Stat for the device path
when generating systemd device properties, to avoid systemd warning for
non-existing devices. The idea was, since systemd uses stat(2) to look
up device properties for a given path, it will fail anyway. In addition,
this allowed to suppress a warning like this from systemd:

> Couldn't stat device /dev/char/10:200

NOTE that this was done because:
 - systemd could not add the rule anyway;
 - runs puts its own set of rules on top of what systemd does.

Apparently, the above change broke some setups, resulting in inability
to use e.g. /dev/null inside a container. My guess is this is because
in cgroup v2 we add a second eBPF program, which is not used if the
first one (added by systemd) returns "access denied".

Next, commit 3b9582895b fixed that by adding a call to os.Stat for
"/sys/"+path (meaning, if "/dev/char/10:200" does not exist, we retry
with "/sys/dev/char/10:200", and if it exists, proceed with adding a
device rule with the original (non-"/sys") path).

How that second fix ever worked was a mystery, because the path we gave
to systemd still doesn't exist.

Well, I think now I know.

Since systemd v240 (commit 74c48bf5a8005f20) device access rules
specified as /dev/{block|char}/MM:mm are no longer looked up on the
filesystem, instead, if possible, those are parsed from the string.

So, we need to do different things, depending on systemd version:

 - for systemd >= v240, use the /dev/{char,block}/MM:mm as is, without
   doing stat() -- since systemd doesn't do stat() either;
 - for older version, check if the path exists, and skip passing it on
   to systemd otherwise.
 - the check for /sys/dev/{block,char}/MM:mm is not needed in either
   case.

Pass the systemd version to the function that generates the rules, and
fix it accordingly.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit d7208f5910)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-25 09:38:05 -07:00
Kir Kolyshkin e4ce94e291 libct/cg: add misc controller to v1 drivers
This is just so that the container can join the misc controller.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 611bbacb3b)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-10 15:34:51 -07:00
Peter Hunt~ 10cfd81631 libctr/cgroups: don't take init's cgroup into account
Sometimes, the init process is not in the root cgroup.
This can be noted by GetInitPath, which already scrubs the path of `init.scope`.

This was encountered when trying to patch the Kubelet to handle systemd being in a separate cpuset
from root (to allow load balance disabling for containers). At present, there's no way to have libcontainer or runc
manage cgroups in a hierarchy outside of the one init is in (unless the path contains `init.scope`, which is limiting)

Signed-off-by: Peter Hunt <pehunt@redhat.com>
(cherry picked from commit 54e20217a8)
2023-04-07 09:41:30 -04:00
Kir Kolyshkin 840b95394b Fix runc run "permission denied" when rootless
Since commit 957d97bcf4 was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: #3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] https://github.com/opencontainers/runc/issues/3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] https://github.com/opencontainers/runc/issues/3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 8491d33482)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-06 18:07:03 -07:00
Chengen, Du 26a58fdb8b cgroups: cpuset: fix byte order while parsing cpuset range to bits
Runc parses cpuset range to bits in the case of cgroup v2 + systemd as cgroup driver.
The byte order representation differs from systemd expectation, which will set
different cpuset range in systemd transient unit if the length of parsed byte array exceeds one.

	# cat config.json
	...
	"resources": {
		...
		"cpu": {
			"cpus": "10-23"
		}
	},
	...
	# runc --systemd-cgroup run test
	# cat /run/systemd/transient/runc-test.scope.d/50-AllowedCPUs.conf
	# This is a drop-in unit file extension, created via "systemctl set-property"
	# or an equivalent operation. Do not edit.
	[Scope]
	AllowedCPUs=0-7 10-15

The cpuset.cpus in cgroup will also be set to wrong value after reloading systemd manager configuration.

	# systemctl daemon-reload
	# cat /sys/fs/cgroup/system.slice/runc-test.scope/cpuset.cpus
	0-7,10-15

Signed-off-by: seyeongkim <seyeong.kim@canonical.com>
Signed-off-by: Chengen, Du <chengen.du@canonical.com>
(cherry picked from commit 77cae9addc)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-06 15:34:31 -07:00
Kir Kolyshkin add2f548fe Merge pull request #3791 from kolyshkin/1.1-refresh-ci
[1.1] refresh ci
2023-04-06 15:34:07 -07:00
Kir Kolyshkin 8d9d1d2502 libct/int: make TestFdLeaks more robust
The purpose of this test is to check that there are no extra file
descriptors left open after repeated calls to runContainer. In fact,
the first call to runContainer leaves a few file descriptors opened,
and this is by design.

Previously, this test relied on two things:
1. some other tests were run before it (and thus all such opened-once
   file descriptors are already opened);
2.  explicitly excluding fd opened to /sys/fs/cgroup.

Now, if we run this test separately, it will fail (because of 1 above).
The same may happen if the tests are run in a random order.

To fix this, add a container run before collection the initial fd list,
so those fds that are opened once are included and won't be reported.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit f2e71b085d)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 10:26:27 -07:00
Kir Kolyshkin b66d6d56b9 libct/int: wording nits
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit be7e03940f)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 10:26:08 -07:00
Kir Kolyshkin ddbb6d4145 libc/int: add/use runContainerOk wrapper
This is to de-duplicate the code that checks that err is nil
and that the exit code is zero.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 7c75e84e22)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 10:25:46 -07:00
Kir Kolyshkin 1f9e36c055 libct: fixes for godoc 1.19
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>
(cherry picked from commit 45cc290f02)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 10:11:22 -07:00
Kir Kolyshkin 77472ef6e0 libct: fix staticcheck warning
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>
(cherry picked from commit 6662570110)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 10:11:22 -07:00
Kir Kolyshkin 9994fe3f19 libct: suppress strings.Title deprecation warning
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>
(cherry picked from commit 7cec81e060)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 10:11:22 -07:00
Kir Kolyshkin 55462355d6 Require Go 1.17, bump x/sys and x/net
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

This reuses parts of main commit a0f8847e2a.

[1] https://github.com/opencontainers/runc/issues/3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 09:55:25 -07:00
Rodrigo Campos 3ce9c1e24b tests: Fix weird error on centos-9
centos-9 unit test sometimes fails with:

	=== RUN   TestPodSkipDevicesUpdate
	    systemd_test.go:114: container stderr not empty: basename: missing operand
	        Try 'basename --help' for more information.
	--- FAIL: TestPodSkipDevicesUpdate (0.11s)

I'm not sure why the container output is an error in basename. It seems
likely that the bashrc in that distro is kind of broken. Let's just run
a sleep command and forget about bash.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
(cherry picked from commit 4d0a60ca7f)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 09:55:25 -07:00
Kir Kolyshkin f6562f19a3 [1.1] libct/cg/dev: skip flaky test of CentOS 7
There is some kind of a race in CentOS 7 which sometimes result in one
of these tests failing like this:

	systemd_test.go:136: mkdir /sys/fs/cgroup/hugetlb/system.slice/system-runc_test_pods.slice: no such file or directory

or

	systemd_test.go:187: open /sys/fs/cgroup/cpuset/system.slice/system-runc_test_pods.slice/cpuset.mems: no such file or directory

As this is only happening on CentOS 7, let's skip this test on this
platform.

[This is a manual cherry-pick of main commit a7a836effa691edb6e.]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 09:14:36 -07:00
Kir Kolyshkin 12f2f03fd2 [1.1] runc run: refuse a non-empty cgroup for systemd driver
As this is currently not possible to add a PID into an existing systemd
unit, plus this feature will be deprected in runc 1.2 (see commit
d08bc0c1b3 ("runc run: warn on non-empty cgroup"), let's reject
sharing a systemd unit between two containers, and fix the test case
accordingly.

We still allow this to happen in case cgroupfs driver is used, to
minimize the potential compatibility issues in a stable branch.

This is an adaptation of main branch commit 82bc89cd10 for 1.1.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-03 15:01:06 -07:00