Commit 4d1d6185ab added this
nsenter_unsupported.go file in order for nsenter to be a valid (but
empty, non-functional) Go package on unsupported platforms.
As a result, runc can be build successfully without CGO, which results
in a non-working and hard-to-debug binary (see issue 3330).
As the functionality of being able to compile a package which is
definitely not working is questionable, and I can't think of any use
cases, let's remove the file.
With this, runc can no longer be build without CGO:
[kir@kir-rhat runc]$ CGO_ENABLED=0 make runc
go build -trimpath "-buildmode=pie" -tags "seccomp" -ldflags "-X main.gitCommit=v1.0.0-452-g00f56786-dirty -X main.version=1.1.0-rc.1+dev " -o runc .
go build github.com/opencontainers/runc/libcontainer/nsenter: build constraints exclude all Go files in /home/kir/go/src/github.com/opencontainers/runc/libcontainer/nsenter
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 12e99a0f8d we do require Go >= 1.16, so this file
is no longer needed.
Also, this actually ensures that go >= 1.16 is used (otherwise
libcontainer/cgroups/getallpids.go won't compile).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The new mount option "rro" makes the mount point recursively read-only,
by calling `mount_setattr(2)` with `MOUNT_ATTR_RDONLY` and `AT_RECURSIVE`.
https://man7.org/linux/man-pages/man2/mount_setattr.2.html
Requires kernel >= 5.12.
The "rro" option string conforms to the proposal in util-linux/util-linux Issue 1501.
Fix issue 2823
Similary, this commit also adds the following mount options:
- rrw
- r[no]{suid,dev,exec,relatime,atime,strictatime,diratime,symfollow}
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
When writing netlink messages, it is possible to have a byte array
larger than UINT16_MAX which would result in the length field
overflowing and allowing user-controlled data to be parsed as control
characters (such as creating custom mount points, changing which set of
namespaces to allow, and so on).
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Since commit 7296dc1712, type intelRdtData is only used by tests,
and since commit 79d292b9f, its only member is config.
Change the test to use config directly, and remove the type.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It never returns any error, so let's drop it (in case it needs to be
re-added, it is easy to do so).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 8850636eb3 (February 2015) this function is no longer
used (replaced by (*ConfigValidator).rootfs), so let's remove it,
together with its unit tests (which were added by commit 917c1f6d6 in
April 2016).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Delegating cgroups to the container enables more complex workloads,
including systemd-based workloads. The OCI runtime-spec was
recently updated to explicitly admit such delegation, through
specification of cgroup ownership semantics:
https://github.com/opencontainers/runtime-spec/pull/1123
Pursuant to the updated OCI runtime-spec, change the ownership of
the container's cgroup directory and particular files therein, when
using cgroups v2 and when the cgroupfs is to be mounted read/write.
As a result of this change, systemd workloads can run in isolated
user namespaces on OpenShift when the sandbox's cgroupfs is mounted
read/write.
It might be possible to implement this feature in other cgroup
managers, but that work is deferred.
Signed-off-by: Fraser Tweedale <ftweedal@redhat.com>
This is not used since commit 5e7b48f7c0 (23 Mar 2017).
In case there are external users, they should switch to
opencontainers/selinux.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Move test case comments to doc strings, and use t.Run.
Suggested-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There is no need to run hooks when `Config.Hooks` is just an empty map,
(dlv) p p.config.Config.Hooks
github.com/opencontainers/runc/libcontainer/configs.Hooks []
Signed-off-by: Dave Chen <dave.chen@arm.com>
1. Instead of distinguishing between errors and warnings, let's treat all
errors as warnings, thus simplifying the code. This changes the
function behaviour for input like hugepages-BadNumberKb --
previously, the error from Atoi("BadNumber") was considered fatal,
now it's just another warnings.
2. Move the warning logging to HugePageSizes, thus simplifying the test
case, which no longer needs to read what logrus writes. Note that we
do not want to log all the warnings (as chances are very low we'll
get any, and if we do this means the code need to be updated), only
the first one.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
I have noticed that libct/cg/fs allocates 8K during init on every runc
execution:
> init github.com/opencontainers/runc/libcontainer/cgroups/fs @1.5 ms, 0.028 ms clock, 8512 bytes, 13 allocs
Apparently this is caused by global HugePageSizes variable init, which
is only used from GetStats (i.e. it is never used by runc itself).
Remove it, and use HugePageSizes() directly instead. Make it init-once,
so that GetStats (which, I guess, is periodically called by kubernetes)
does not re-read huge page sizes over and over.
This also removes 12 allocs and 8K from libct/cg/fs init section:
> $ time GODEBUG=inittrace=1 ./runc --help 2>&1 | grep cgroups/fs
> init github.com/opencontainers/runc/libcontainer/cgroups/fs @1.5 ms, 0.003 ms clock, 16 bytes, 1 allocs
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Since GetHugePageSize do not have any external users (checked by
sourcegraph), and no internal user ever uses its second return value
(the error), let's drop it.
2. Rename GetHugePageSize -> HugePageSizes (drop the Get prefix as per
Go guidelines, add suffix since we return many sizes).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Using null bytes as control characters for sending strings via netlink
opens us up to a user explicitly putting a null byte in a mount string
(which JSON will happily let you do) and then causing us to open a mount
path different to the one expected.
In practice this is more of an issue in an environment such as
Kubernetes where you may have path-based access control policies (which
are more susceptible to these kinds of flaws).
Found by Google Project Zero.
Fixes: 9c444070ec ("Open bind mount sources from the host userns")
Reported-by: Felix Wilhelm <fwilhelm@google.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
As reported in [1], in a case where read-only fuse (sshfs) mount
is used as a volume without specifying ro flag, the kernel fails
to remount it (when adding various flags such as nosuid and nodev),
returning EPERM.
Here's the relevant strace line:
> [pid 333966] mount("/tmp/bats-run-PRVfWc/runc.RbNv8g/bundle/mnt", "/proc/self/fd/7", 0xc0001e9164, MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC, NULL) = -1 EPERM (Operation not permitted)
I was not able to reproduce it with other read-only mounts as the source
(tried tmpfs, read-only bind mount, and an ext2 mount), so somehow this
might be specific to fuse.
The fix is to check whether the source has RDONLY flag, and retry the
remount with this flag added.
A test case (which was kind of hard to write) is added, and it fails
without the fix. Note that rootless user need to be able to ssh to
rootless@localhost in order to sshfs to work -- amend setup scripts
to make it work, and skip the test if the setup is not working.
[1] https://github.com/containers/podman/issues/12205
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The staticcheck linter points out that the err != nil comparison
after system.Exec is always true:
> libcontainer/standard_init_linux.go#L253
> SA4023: this comparison is always true (staticcheck)
> libcontainer/system/linux.go#L43
> SA4023(related information): github.com/opencontainers/runc/libcontainer/system.Exec never returns a nil interface value (staticcheck)
Indeed, Exec either returns an error or does not return at all.
Remove the (useless) check.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Decapitalize errors.
2. Rename isValidName to checkPropertyName.
3. Make it return a specific error.
Suggested-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit fb4c27c4b7 (went into v1.0.0-rc93) fixed a bug with
read-only tmpfs, but introduced a bug with read-only /dev.
This happens because /dev is a tmpfs mount and is therefore remounted
read-only a bit earlier than before.
To fix,
1. Revert the part of the above commit which remounts all tmpfs mounts
as read-only in mountToRootfs.
2. Reuse finalizeRootfs (which is already used to remount /dev
read-only) to also remount all ro tmpfs mounts that were previously
mounted rw in mountPropagate.
3. Remove the break in finalizeRootfs, as now we have more than one
mount to care about.
4. Reorder the if statements in finalizeRootfs to perform the fast check
(for ro flag) first, and compare the strings second. Since /dev is
most probably also a tmpfs mount, do the m.Device check first.
Add a test case to validate the fix and prevent future regressions;
make sure it fails before the fix:
✗ runc run [ro /dev mount]
(in test file tests/integration/mounts.bats, line 45)
`[ "$status" -eq 0 ]' failed
runc spec (status=0):
runc run test_busybox (status=1):
time="2021-11-12T12:19:48-08:00" level=error msg="runc run failed: unable to start container process: error during container init: error mounting \"devpts\" to rootfs at \"/dev/pts\": mkdir /tmp/bats-run-VJXQk7/runc.0Fj70w/bundle/rootfs/dev/pts: read-only file system"
Fixes: fb4c27c4b7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Also, add a simple test and a benchmark (just out of sheer curiosity).
Benchmark results:
name old time/op new time/op delta
IsValidName-4 540ns ± 3% 45ns ± 1% -91.76% (p=0.008 n=5+5)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 1cd71dfd7 added isSecSuffix, but the same thing can be done
easily without a regex. This is faster and saves some init time and
memory.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
parseMountOption already returns way too many values, making the code
kind of hard to read.
Since all of the return values are used as is to populate the fields of
configs.Mount, let's change it to return (semi-)populated *configs.Mount
instead.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This makes the repeated calls to parseMountOptions faster,
and decreases the amount of garbage to collect.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These two maps are the same, except that mountPropagationMapping
has an extra element with key of "" and value of 0. Since the
code already checks for f != 0, this extra element is not a problem.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Eliminate some of these allocations when starting runc:
> init github.com/opencontainers/runc/libcontainer/specconv @10 ms, 0.11 ms clock, 5408 bytes, 70 allocs
Most of this (4K) is the two regexes, which are left intact for now.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When I tried to start a rootless container under a different/wrong user,
I got:
$ ../runc/runc --systemd-cgroup --root /tmp/runc.$$ run 445
ERRO[0000] runc run failed: operation not permitted
This is obviously not good enough. With this commit, the error is:
ERRO[0000] runc run failed: fchown fd 9: operation not permitted
Alas, there are still some code that returns unwrapped errnos from
various unix calls.
This is a followup to commit d8ba4128b2 which wrapped many, but not
all, bare unix errors. Do wrap some more, using either os.PathError or
os.SyscallError.
While at it,
- use os.SyscallError instead of os.NewSyscallError;
- use errors.Is(err, os.ErrXxx) instead of os.IsXxx(err).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes a container cgroup already exists but is frozen.
When this happens, runc init hangs, and it's not clear what is going on.
Refuse to run in a frozen cgroup; add a test case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently runc allows multiple containers to share the same cgroup (for
example, by having the same cgroupPath in config.json). While such
shared configuration might be OK, there are some issues:
- When each container has its own resource limits, the order of
containers start determines whose limits will be effectively applied.
- When one of containers is paused, all others are paused, too.
- When a container is paused, any attempt to do runc create/run/exec
end up with runc init stuck inside a frozen cgroup.
- When a systemd cgroup manager is used, this becomes even worse -- such
as, stop (or even failed start) of any container results in
"stopTransientUnit" command being sent to systemd, and so (depending on
unit properties) other containers can receive SIGTERM, be killed after a
timeout etc.
Any of the above may lead to various hard-to-debug situations in production
(runc init stuck, cgroup removal error, wrong resource limits, init not
reaping zombies etc.).
One obvious solution is to refuse a non-empty cgroup when starting a new
container. This would be a breaking change though, so let's make it in
steps, with the first step is issue a warning and a deprecated notice
about a non-empty cgroup.
Later (in runc 1.2) we will replace this warning with an error.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Runtime spec says:
> sysctl (object, OPTIONAL) allows kernel parameters to be modified at
> runtime for the container. For more information, see the sysctl(8)
> man page.
and sysctl(8) says:
> variable
> The name of a key to read from. An example is
> kernel.ostype. The '/' separator is also accepted in place of a '.'.
Apparently, runc config validator do not support sysctls with / as a
separator. Fortunately this is a one-line fix.
Add some more test data where / is used as a separator.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin