This reverts commit 814f3ae1d9. This
changed the on-disk state which breaks runc when it has to operate on
containers started with an older runc version. Working around this is
far more complicated than just reverting it.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Looking into data generated by setting
GODEBUG="inittrace=1"
I have noticed this line:
init github.com/opencontainers/runc/libcontainer/cgroups/devices @1.2 ms, 0.020 ms clock, 10512 bytes, 133 allocs
This is the leader for both bytes and allocs among the packages from
this repo, and all of it is caused by a single regex:
> var devicesListRegexp = regexp.MustCompile(`^([abc])\s+(\d+|\*):(\d+|\*)\s+([rwm]+)$`)
It seems that the same parsing can be done without relying on
a regular expression, no decrease in readability, and 2x faster
(according to the benchmark added), and also makes runc start
slightly faster and leaner.
Before:
BenchmarkParseLine-4 176240 6768 ns/op 6576 B/op 64 allocs/op
After:
BenchmarkParseLine-4 322441 3535 ns/op 5520 B/op 53 allocs/op
[v2: single split with SplitFunc; fix a typo in error message]
[v3: rebase after 3159 merge; re-ran benchmarks (results are similar)]
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is helpful to kubernetes in cases it knows for sure that the freeze
is not required (since it created the systemd unit with no device
restrictions).
As the code is trivial, no tests are required.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a test for freezeBeforeSet, checking various scenarios including
those that were failing before the fix in the previous commit.
[v2: add more cases, add a check before creating a unit.]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the behavior intended to avoid freezing containers/control
groups without it being necessary. This is important for end users of
libcontainer who rely on the behavior of no freeze.
The previous implementation would always get error trying to get
DevicePolicy from the Unit via dbus, since the Unit interface doesn't
contain DevicePolicy.
Signed-off-by: Odin Ugedal <odin@uged.al>
The two exceptions I had to add to codespellrc are:
- CLOS (used by intelrtd);
- creat (syscall name used in tests/integration/testdata/seccomp_*.json).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 2bab4a5 resulted in a warning from gcc:
nsexec.c: In function ‘write_log’:
nsexec.c:171:2: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
171 | write(logfd, json, ret);
| ^~~~~~~~~~~~~~~~~~~~~~~
As there's nothing we can or want to do in case write fails,
let's just tell the compiler we're not going to use it.
Fixes: 2bab4a5
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Today we support the seccomp build tag only that is used by default.
However, we are not testing that compiling without any build tag works.
I found the CI didn't catch this when working on #2682, that the CI was
green but compilation without build tags was broken.
We test compilation without build tags only, compilation with the only
build tag supported is done extensively in other actions.
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
The contents of the pointer returned on asprintf() error are undefined
i.e., it can be anything there. We set it to NULL on error so that
free() afterwards won't get a garbage pointer.
This patch applies the above to message and stage as well to be
consistent with what we do for json.
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
According to C standards, `size_t` is always an unsigned integer type.
Thus, checking unsigned expressions to be less than zero is not needed.
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
Possibly there was a specific reason to use a rune for this, but I noticed
that there's various parts in the code that has to convert values from a
string to this type. Using a string as type for this can simplify some of
that code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
GCP images description at [1] claims that:
- For CentOS 8 and CentOS Stream 8, the PowerTools repository is
enabled.
- For CentOS 7, EPEL is enabled.
Apparently,
- we do not need epel for centos-stream-8;
- powertools is not enabled on centos-stream-8 despite [1].
Anyway, the less yum commands the better, as we have seen those fail
sometimes due to occasional networking problems etc.
[1] https://cloud.google.com/compute/docs/images/os-details#centos
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported in issue 3119, there is a race in nsexec logging
that can lead to garbled json received by log forwarder, which
complains about it with a "failed to decode" error.
This happens because dprintf (used since the very beginning of nsexec
logging introduced in commit ba3cabf932) relies on multiple write(2)
calls, and with additional logging added by 64bb59f592 a race is
possible between runc init parent and its children.
The fix is to prepare a string and write it using a single call to
write(2).
[v2: NULLify json on error from asprintf]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin (3):
libct/cg: GetAllPids: optimize for go 1.16+
libct/cg: improve GetAllPids and readProcsFile
libct/cg: move GetAllPids out of utils.go
LGTMs: AkihiroSuda cyphar
Closes#3133
filepath.WalkDir function, introduced in Go 1.16, doesn't do stat(2)
on every entry, and is therefore somewhat faster (see below).
Since we have to support Go 1.15, keep the old version for backward
compatibility.
Add a quick benchmark, which shows approximately 3x improvement:
$ go1.15.15 test -bench AllPid -run xxx .
BenchmarkGetAllPids-4 48 23528839 ns/op
$ go version
go version go1.16.6 linux/amd64
$ go test -bench AllPid -run xxx .
BenchmarkGetAllPids-4 147 7700170 ns/op
(Unrelated but worth noting -- go 1.17rc2 is pushing it even further)
$ go1.17rc2 test -bench AllPid -run xxx .
BenchmarkGetAllPids-4 164 6820994 ns/op
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since every cgroup directory is guaranteed to have cgroup.procs file,
we don't have to do filename comparison in GetAllPids() and just read
cgroup.procs in every directory.
While at it, switch readProcsFile to use our own OpenFile.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add testing against criu-dev branch instead of a released version
(happens to be criu v3.15 at the moment), to check how it works.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently there's no way to distinguish between the two cases:
- runc exec failed;
- the command executed returned 1.
This was possible before commit 8477638aab, as runc exec exited with
the code of 255 if exec itself has failed. The code of 255 is the same
convention as used by e.g. ssh.
Re-introduce the feature, document it, and add some tests so it won't be
broken again.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
What it takes is add an empty buildid, which, together with previously
added strip invocation, results in reproducible build!
NB: earlier versions of this patch also added the following:
1. non-random libseccomp install $prefix;
2. "objcopy --enable-deterministic-archives $prefix/lib/libseccomp.a"
to strip ar dates and UIDs/GIDs;
3. "-B=0x00" to EXTRA_LDFLAGS to have non-variable NT_GNU_BUILD_ID.
Apparently, all this is not needed with strip.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Aleksa Sarai