Commit Graph

4974 Commits

Author SHA1 Message Date
Kir Kolyshkin d0cbef576f Makefile: rm go 1.13 workaround
Since go 1.14, mod=vendor is used automatically. Since go 1.16 is now
released, and minimally supported go version is 1.15.

As per commit fbeed5228, remove the go 1.13 workaround.

Fix README to require go 1.14.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-16 18:58:55 -08:00
Kir Kolyshkin 4019f08d68 make validate: rm go vet
As we already run go vet during CI (via golangci-lint) let's not run it
again.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-16 18:58:55 -08:00
Kir Kolyshkin f9c2113331 make lint: use golangci-lint
Instead of running just a couple of linters, use the same set as we run
on CI.

NOTE that this is for developers, not scripts, and requires having
golangci-lint installed (see
https://golangci-lint.run/usage/install/#local-installation).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-16 18:58:55 -08:00
Kir Kolyshkin 671bb97825 Makefile: remove ci target
It is not used since commit e431fe60f8.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-16 18:58:55 -08:00
Kir Kolyshkin 9594085573 script/validate-gofmt: rm
Add gofmt to golangci-lint configuration, and remove the script
which ran gofmt.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-16 18:58:55 -08:00
Akihiro Suda f245a1d1ed Merge pull request #2808 from kolyshkin/go116-final 2021-02-17 10:50:03 +09:00
Mrunal Patel 06c1cd8896 Merge pull request #2806 from kolyshkin/readme-gha-badges
README: add gha badges
2021-02-16 14:50:30 -08:00
Kir Kolyshkin 91f0ae1884 ci/gha: bump go 1.16-rc1 -> 1.16.x
As the final go 1.16 is released, rc1 is no longer available.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-16 12:30:05 -08:00
Kir Kolyshkin 5b14a26164 README: add gha badges
Note that validate currently fails on master -- this is tracked
in https://github.com/opencontainers/runc/issues/2627.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-10 14:39:27 -08:00
Mrunal Patel 824e4ad3b8 Merge pull request #2803 from AkihiroSuda/fix-arch-apparmor
apparmor: try attr/apparmor/exec before attr/exec
2021-02-10 10:29:13 -08:00
Akihiro Suda 9f1365373d Merge pull request #2796 from vasiliy-ul/ebpf-fix-device-access-check
ebpf: fix device access check
2021-02-10 16:37:01 +09:00
Akihiro Suda f3f563bc0f apparmor: try attr/apparmor/exec before attr/exec
Fix issue 2801

Tested on Arch Linux with the following configuration
```
[root@archlinux ~]# pacman -Q runc containerd docker linux
runc 1.0.0rc93-1
containerd 1.4.3-1
docker 1:20.10.3-1
linux 5.10.14.arch1-1

[root@archlinux ~]# cat /proc/cmdline
BOOT_IMAGE=/boot/vmlinuz-linux root=UUID=bd8aaf58-8735-4fd5-a0c1-804a998f8d57 rw net.ifnames=0 rootflags=compress-force=zstd apparmor=1 lsm=capability,lockdown,yama,bpf,apparmor

[root@archlinux ~]# cat /etc/docker/daemon.json
{
  "runtimes": {
    "runc-tmp": {
      "path": "/tmp/runc"
    }
  }
}

[root@archlinux ~]# docker run -it --rm alpine
docker: Error response from daemon: OCI runtime create failed: container_linux.go:367: starting container process caused: process_linux.go:495: container init caused: apply apparmor profile: apparmor failed to apply profile: write /proc/self/attr/exec: invalid argument: unknown.

[root@archlinux ~]# docker run -it --rm --runtime=runc-tmp alpine
/ # cat /proc/self/attr/apparmor/current
docker-default (enforce)
/ # cat /proc/self/attr/current
cat: read error: Invalid argument
``

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2021-02-10 16:27:13 +09:00
Mrunal Patel 2dbfa87477 Merge pull request #2793 from cyphar/cgroup2-ebpf-userns
cgroup2: devices: handle eBPF skipping more correctly
2021-02-09 15:12:12 -08:00
Vasiliy Ulyanov efb8552b05 tests/int: add device access test
The test verifies if the device file can be queried using
'access(dev_name, F_OK)' when the permissions are set to 'rw'. The call
does not explicitly trigger read or write access but should succeed.

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
2021-02-08 14:14:14 +01:00
Vasiliy Ulyanov 81707abd33 ebpf: fix device access check
Checking the access mode as bellow

    if (R3 & bpfAccess == 0 /* use R1 as a temp var */) goto next

does not handle correctly device file probing with:

    access(dev_name, F_OK)

F_OK does not trigger read or write access. Instead the access type in
R3 in that case will be zero and the check will not pass even if "rw" is
allowed for the device file. Comparing the 'masked' access type with the
requested one solves the issue:

    if (R3 & bpfAccess != R3 /* use R1 as a temp var */) goto next

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
2021-02-07 19:33:10 +01:00
Aleksa Sarai 94ae7afa36 merge branch 'pr-2791'
Kir Kolyshkin (1):
  cgroupv1 freezer: thaw to increase freeze chances

LGTMs: @cyphar @dqminh
Closes #2791
2021-02-07 19:04:04 +11:00
Aleksa Sarai 2831fb5595 cgroup2: devices: handle eBPF skipping more correctly
In the past we incorrectly handled eBPF errors in two ways:

 1. We would only ignore errors if there was an allow rule in the list
    (this doesn't make sense because for security purposes we only care
    if a *deny* rule is being ignored). Arguably this is a security flaw
    but you would only get an error from bpf(2) in rare cases, and thus
    is not a big enough deal to go through security review.

 2. If we were in a rootless container we would still return an error
    even though bpf(2) is blocked for rootless containers.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2021-02-05 18:13:06 +11:00
Aleksa Sarai b4cb54c2ea merge branch 'pr-2782'
Kir Kolyshkin (2):
  ci/gha: bump golangci-lint to v1.36
  .golangci.yml: add

LGTMs: @AkihiroSuda @cyphar
Closes #2782
2021-02-05 17:46:52 +11:00
Akihiro Suda 9f1e43ea69 Merge pull request #2741 from kolyshkin/test-images 2021-02-05 15:30:43 +09:00
Akihiro Suda 072ed8779b Merge pull request #2789 from kolyshkin/error-trace 2021-02-05 15:21:56 +09:00
Kir Kolyshkin d1007b08a3 cgroupv1 freezer: thaw to increase freeze chances
It appears that briefly thawing the cgroup while freezing
greatly increases its chances to freeze successfully.

The test case I used is doing runc exec in a look parallel with runc
pause/resume in another loop, and the failure to freeze rate reduced
from 40 to 0 per minute (tested inside a VM using a busybox container
running sleep 1h, doing about 1500 pause/resumes and 650 execs per
minute), with max retries being 150 (of 1000).

This is still a game of chances, so failures are possible.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-04 15:06:11 -08:00
Kir Kolyshkin 1e0016cf57 Show error stack trace if --debug is set
We use github.com/pkg/errors to produce an error in many places.
One of its benefits is error comes with a stack trace.

Let's print that stack trace if --debug is set.

Example:

	# ../runc --debug run -d ''
	ERRO[0000] container id cannot be empty
	DEBU[0000] container id cannot be empty
	main.init
		github.com/opencontainers/runc/utils_linux.go:28
	runtime.doInit
		runtime/proc.go:5652
	runtime.main
		runtime/proc.go:191
	runtime.goexit
		runtime/asm_amd64.s:1374

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-04 11:50:21 -08:00
Mrunal Patel 19437f3a55 Merge pull request #2787 from kolyshkin/gha-fedora
ci/gha/fedora: retry vagrant up
2021-02-03 17:41:34 -08:00
Mrunal Patel c46eced395 Merge pull request #2788 from kinvolk/mauricio/fix_err_conversion
libcontainer: fix bad conversion from syscall.Errno to error
2021-02-03 17:40:52 -08:00
Aleksa Sarai 7c11557a07 merge branch 'pr-2784'
Aleksa Sarai (2):
  VERSION: back to development
  VERSION: release 1.0.0~rc93

Vote: +6 -0 #1
Closes #2784
2021-02-04 11:15:41 +11:00
Mauricio Vásquez 5c0342ba2c libcontainer: fix bad conversion from syscall.Errno to error
The correct way to do that conversion according to
https://pkg.go.dev/syscall#Errno is:

```
err = nil
if errno != 0 {
	err = errno
}
```

In this case the error check will always report a false positive in
unix.RawSyscall(unix.SYS_SECCOMP, ...), probably nobody has faced this
problem because the code takes the other path in most of the cases.

Fixes: 7a8d7162f9 ("seccomp: prepend -ENOSYS stub to all filters")

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
2021-02-03 16:12:33 -05:00
Kir Kolyshkin a9e99b6da3 ci/gha/fedora: retry vagrant up
download.fedoraproject.org gives HTTP 404 at times,
breaking the CI. Let's give it another chance.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-03 10:56:00 -08:00
Aleksa Sarai f26768a820 VERSION: back to development
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2021-02-03 11:58:14 +11:00
Aleksa Sarai 12644e614e VERSION: release 1.0.0~rc93
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
v1.0.0-rc93
2021-02-03 11:57:49 +11:00
Kir Kolyshkin c348b98203 tests/rootless.sh: fix/ignore shellcheck warnings
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-02 16:03:52 -08:00
Kir Kolyshkin 1143759338 tests/rootless.sh: use set -e -u -o pipefail
Currently, set -e ("exit on error") is only set before running tests.
The bad consequence is the script may fail to set up test environment
but will still run the tests.

Use "set -e -u -o pipefail" for the whole script to catch potential
issues with test setup and/or the script itself.

A few caveats:

1. We have to set RUNC_USE_SYSTEMD to an empty value if it is unset, so
   that the subsequent checks like [ -z "$RUNC_USE_SYSTEMD" ] won't
   trigger a "unbound variable" error. Same for ROOTLESS_TESTPATH.

2. Functions that have code like [ -f $file ] && do_something towards
   the end may return 1 in case $file does not exist (as test aka [
   was the last operation performed, and its exit code is returned.
   This is why we had to add return 0.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-02 16:03:52 -08:00
Kir Kolyshkin 5ab0588432 ci: untangle getting test images
This simplifies and optimizes getting container images used for tests.

Currently, we have three different ways of getting images:

1. (for hello-world) the image is in this repo under tests/integration/testdata.

2. (for busybox) download it from github (the repo that is used for
   preparing official Docker image) using curl.

3. (for debian) download from Docker hub, using skopeo and umoci.

To further complicate things, we have to do this downloading in multiple
scenarios (at least 4): locally, in github CI, from Dockefile, inside a
Vagrant VM. For each scenario, we have to install skopeo and umoci, and
those two are not yet universally available for all the distros that we
use.

Yet another complication is those images are used for tests/integration
(bats-driven tests) as well as for libcontainer/integration (go tests).
The tests in libcontainer/integration rely on busybox being available
from /busybox, and the bats tests just download the images to a
temporary location during every run.

It is also hard to support CI for other architectures, because all
the machinery for preparing images is so complicated.

This commit is an attempt to simplify and optimize getting images,
mostly by getting rid of skopeo and umoci dependencies, but also
by moving the download logic into one small shell script, which
is used from all the places.

Benefits:

 - images (if not present) are only downloaded once;
 - same images are used for both kind of tests (go and bats);
 - same images are used for local and inside-docker tests
   (because source directory is mounted into container);
 - the download logic is located within 1 simple shell script.

[v2: fix eval; more doc to get-images; print URL if curl failed]
[v3: use "slim" debian, twice as small]
[v4: fix not using $image in setup_bundle]
[v5: don't remove TESTDATA from helpers.bash]
[v6: add i386 support]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-02 16:03:29 -08:00
Kir Kolyshkin bc175ba423 tests/helpers.bash: rm GOPATH
It is not used anywhere since commit 30601efa81.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-02 11:47:06 -08:00
Aleksa Sarai 7e3c3e8c22 merge branch 'pr-2780'
Sebastiaan van Stijn (1):
  Remove "PatchAndLoad" stub as it's not used without seccomp enabled

LGTMs: @kolyshkin @cyphar
Closes #2780
2021-02-03 00:45:32 +11:00
Aleksa Sarai cc988c1036 merge branch 'pr-2774'
Kir Kolyshkin (1):
  libct/cg/fs/freezer: fix freezing race

LGTMs: @mrunalp @cyphar
Closes #2774
2021-02-02 10:25:35 +11:00
Kir Kolyshkin dc0257926a ci/gha: bump golangci-lint to v1.36
Changes: https://github.com/golangci/golangci-lint/releases

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-01 14:24:15 -08:00
Kir Kolyshkin 4d3a8d5e53 .golangci.yml: add
This is to enable seccomp build tag (some linters do use tags,
some don't).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-01 14:20:59 -08:00
Kir Kolyshkin 76ae1f5c31 libct/cg/fs/freezer: fix freezing race
Before this commit, Set() used GetState() to check the freezer state
and retry the operation if the actual state still differs from requested.
This should help with the situation when a new process (such as one
added by runc exec) is added to the container's cgroup while it's being
freezed by the kernel, but it's not working as it should.

The problem is, GetState() never returns FREEZING state, looping until
the state is either FROZEN or THAWED, so Set() does not have a chance
to repeate the freeze attempt.

As a result, the container might end up stuck in a FREEZING state,
with GetState() never returning (which in turn blocks some other
operations).

One way to fix this would be to have GetState returning FREEZING state
instead of retrying ad infinitum. It would result in changing the public
API, and no callers of GetState expects it to return this.

To fix, let's not use GetState() from Set(). Instead, read the
freezer.state file directly and act accordingly -- return success
on FROZEN, retry on FREEZING, and error out on any other (unexpected)
value.

While at it, further improve the code:
 - limit the number of retries;
 - if retries are exceeded, thaw and return an error;
 - don't retry (or read the state back) on THAW.

I played a lot with various reproducers for this bug, including

 - parallel runc execs and runc pause/resumes
 - parallel runc execs and runc --systemd-cgroup update
   (the latter performs freeze/unfreeze);
 - continuously running /bin/printf inside container
   in parallel with runc pause/resume;
 - running pthread bomb (from criu test suite) in parallel
   with runc pause/resume;

and I was not able to make freeze work 100%, meaning sometimes
runc pause fails, or runc --systemd-cgroup update produces a warning.

With that said, it's still a big improvement over the previous
state of affairs where container is stuck in FREEZING state,
and GetState() (and all its users) are also stuck.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-01 13:54:00 -08:00
Sebastiaan van Stijn c4bc3b080e Remove "PatchAndLoad" stub as it's not used without seccomp enabled
This function is called by `InitSeccomp`, but only when compiled
with seccomp (and cgo) enabled, so should not be needed for other
situations.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-02-01 22:01:59 +01:00
Kir Kolyshkin 6c85f6389e Merge pull request #2775 from cyphar/fix-build
cgroupfs: cpuset: fix broken build
2021-02-01 09:02:21 -08:00
Aleksa Sarai 4074b47975 merge branch 'pr-2636'
Kenta Tada (1):
  libcontainer/intelrdt: adjust the file mode

LGTMs: @AkihiroSuda @cyphar
Closes #2636
2021-02-01 20:50:25 +11:00
Akihiro Suda 2046f264ca Merge pull request #2755 from kolyshkin/numa-stat
libct/cg/fs: getPageUsageByNUMA: rewrite/optimize, fix panic, add more tests
2021-02-01 13:49:53 +09:00
Aleksa Sarai 6ddfaa5e95 cgroupfs: cpuset: fix broken build
The merge 6eed6e5795 broke the build because ab27e12ceb ("Implement
GetStat for cpuset cgroup.") dropped the errors import which was used by
c85cd2b325 ("libct/cg/fs/cpuset: don't parse mountinfo") and the CI
wasn't retriggered.

Fix this by just importing "github.com/pkg/errors" again.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
2021-02-01 14:31:47 +11:00
Aleksa Sarai 091dd32dd1 merge branch 'pr-2607'
Sebastiaan van Stijn (1):
  libcontainer: move capabilities to separate package

LGTMs: @AkihiroSuda @cyphar
Closes #2607
2021-02-01 14:26:17 +11:00
Aleksa Sarai a4f2b2b0a8 merge branch 'pr-2769'
Kir Kolyshkin (2):
  ci: test with golang 1.16-rc1
  tests/int/spec.bats: fix for go 1.16

LGTMs: @AkihiroSuda @cyphar
Closes #2769
2021-02-01 11:05:49 +11:00
Aleksa Sarai 6eed6e5795 merge branch 'pr-2599'
Kir Kolyshkin (4):
  libct/cgroups/fs/cpuset: don't use MkdirAll
  libct/cg/fs/cpuset: don't parse mountinfo
  libct/cg/fs.getCgroupRoot: reuse (cached) cgroup mountinfo
  libct/cgroups/v1_utils: implement mountinfo cache

LGTMs: @AkihiroSuda @cyphar
Closes #2599
2021-02-01 11:04:11 +11:00
Kir Kolyshkin c531a6f26a Merge pull request #2750 from cyphar/seccomp-patched-bpf
seccomp: prepend -ENOSYS stub to all filters
2021-01-31 11:54:41 -08:00
Aleksa Sarai e17b96d7b4 merge branch 'pr-2773'
An Long (1):
  Makefile: add -trimpath go build flag

LGTMs: @AkihiroSuda @cyphar
Closes #2773
2021-01-30 16:50:11 +11:00
An Long ca42289658 Makefile: add -trimpath go build flag
Signed-off-by: An Long <aisk1988@gmail.com>
2021-01-30 00:51:03 +08:00
Akihiro Suda e7bd1fb10a Merge pull request #2717 from kolyshkin/check-proc-opt
libct/checkProcMounts: optimize
2021-01-29 17:32:45 +09:00