The same attribute was generated for each path that was created, but always
the same, so instead of generating it in each iteration, generate it once,
and pass it to our mkdirall() implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The regex only matched volume paths without a trailing path-separator. In cases
where a path would be passed with a trailing path-separator, it would depend on
further code in mkdirall to strip the trailing slash, then to perform the regex
again in the next iteration.
While regexes aren't ideal, we're already executing this one, so we may as well
use it to match those situations as well (instead of executing it twice), to
allow us to return early.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Ideally, we would construct this lazily, but adding a function and a
sync.Once felt like a bit "too much".
Also updated the GoDoc for some functions to better describe what they do.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was forked from Moby in 6089c1525b,
which copied the whole file, but the `IsAbs()` was never used, and has no
external consumers, so let's remove it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ForceRemoveAll was only used in tests/fuzzing, but added hcsshim as dependency
for the sys package. Moving this to integration/client makes the "sys" package
slightly more lightweight, and may help simplifying dependency-management.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These functions were originally copied from the docker / moby repository in
4a7a8efc2d. Migrating these functions to use the
github.com/moby/sys/sequential module allows them being shared between moby,
docker/cli, and containerd, and to allow using them without importing all of sys
which also depends on hcsshim and more.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The channel is created with no capacity that it needs receiver when
sending data. Otherwise, the sending-data goroutine will be blocked
forever. For the #6166 pr, the exec command timeout will return and
no receiver for the data. It will cause goroutine leaky.
This commit allocates buffered channel for the command status and closes
the channel after sending. And also use time.Timer with Stop for
performance concern.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Some layers might be prefixed with rm-, which will result
in an error when converting that string into an integer.
Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
The io/ioutil package has been deprecated as of Go 1.16, see
https://golang.org/doc/go1.16#ioutil. This commit replaces the existing
io/ioutil functions with their new definitions in io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Go 1.15.7 contained a security fix for CVE-2021-3115, which allowed arbitrary
code to be executed at build time when using cgo on Windows. This issue also
affects Unix users who have “.” listed explicitly in their PATH and are running
“go get” outside of a module or with module mode disabled.
This issue is not limited to the go command itself, and can also affect binaries
that use `os.Command`, `os.LookPath`, etc.
From the related blogpost (ttps://blog.golang.org/path-security):
> Are your own programs affected?
>
> If you use exec.LookPath or exec.Command in your own programs, you only need to
> be concerned if you (or your users) run your program in a directory with untrusted
> contents. If so, then a subprocess could be started using an executable from dot
> instead of from a system directory. (Again, using an executable from dot happens
> always on Windows and only with uncommon PATH settings on Unix.)
>
> If you are concerned, then we’ve published the more restricted variant of os/exec
> as golang.org/x/sys/execabs. You can use it in your program by simply replacing
This patch replaces all uses of `os/exec` with `golang.org/x/sys/execabs`. While
some uses of `os/exec` should not be problematic (e.g. part of tests), it is
probably good to be consistent, in case code gets moved around.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove build tags which are already implied by the name of the file.
Ensures build tags are used consistently
Signed-off-by: Derek McGowan <derek@mcg.dev>
It's the only location this is used, so might as well move it
into that package. I could not find external users of this utility,
so not adding an alias / deprecation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This removes the StatAtime(), StatCtime(), StatMtime() and StatATimeAsTime()
utilities from the sys package. These utilities are not currently used in the
containerd codebase, and a copy of these utilities can be found in
github.com/containerd/continuity/fs;
- fa7d162237/fs/stat_darwinbsd.go
- fa7d162237/fs/stat_linuxopenbsd.go
Given that the containerd/sys package is not designed for external consumption,
I'm not adding "deprecated" comments (or aliases to their new location).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ForceRemoveAll has special logic on Windows for cleaning up a Windows
snapshotter directory. The logic was missing proper handling for an
error case that can be returned in some cases.
This fixes the CI failure seen in #5326.
Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
(this was introduced in 44240116aa)
Setting the oom-score-adj to -1000 is only possible if the parent process
either has no score set, or if the score is set to -1000.
However, the current implementation of GetOOMScoreAdj conflates situations
where either _no_ oom-score is set, _or_ oom-score is set, but set to 0.
In the latter case, the test would fail:
--- FAIL: TestSetOOMScoreBoundaries (0.01s)
oom_linux_test.go:79: assertion failed: 0 (adjustment int) != -1000 (OOMScoreAdjMin int)
FAIL
To prevent failures in this situation, skip that part of the test in the
above situation.
Also update the description of GetOOMScoreAdj to clarify its behavior.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The shim.SetScore() utility was no longer used since 7dfc605fc6.
Checking for uses outside of this repository, I found only one external use of
this in gVisor; a9441aea27/pkg/shim/service.go (L262-L264)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
SetOOMScore requires both privileged (root) and non-user namespace,
for negative values, so adjust the pre-conditions accordingly.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These are currently only used inside this package, so we might
as well un-export them until we need them elsewhere.
Also updated SetOOMScore() to first check for privileged; check for privileged
looks to be the "faster" path, and checking it first could (in case of non-
privileged) save having to read and parse /proc/self/uid_map.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This ensures that we do not trigger assertions inside HCS by tring to
call hcsshim.DestroyLayer on the parent of a currently-activated layer.
It also deactivates the layers before deletion, to ensure we trigger or
avert file-in-use failures due to leftover state from the tests with
more detail than 'destroy failed'.
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
oom_score_adj must be in the range -1000 to 1000. In AdjustOOMScore if containerd's score is already at the maximum value we should set that value for the shim instead of trying to set 1001 which is invalid.
Signed-off-by: Simon Kaegi <simon_kaegi@ca.ibm.com>
GitHub Actions process wrapper sets score adj to 500 for any process;
the OOM score adj test expected default adj to be 0 during test.
Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
Dependencies may be switching to use the new `%w` formatting
option to wrap errors; switching to use `errors.Is()` makes
sure that we are still able to unwrap the error and detect the
underlying cause.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Given that we're only interested in detecting if userns is
enabled, and no further details about the mapping, we can
revert this function to go back to its original implementation
in github.com/lxc/lxd/shared/util.go
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
After e115b52ce2 was merged, this
function was no longer in use, so we should be able to remove it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It was only used in sys/reaper, and after moving it, that package
no longer depends on the `sys` package.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
forkAndMountat forks a process to chdir then mount layers. Signals are
blocked (using runtime_beforeFork) during fork.
There is a race condition that the child process finishes before the
parent process is scheduled and can unblock signal handling. The SIGCHLD
signal sent from the finished process may have been delivered to the
shim process's reaper thread and caused the parent process fail with
ECHLD error.
This patch sets up a pipe for communication between child and parent
instead of waiting for child exit status.
Fixes#4009.
Signed-off-by: Haitao Li <hli@atlassian.com>
- preserve `syscall.ENOTDIR` to match os.MkDirAll
- change back parameter name to `adminAndLocalSystem`
- revert accidental change of permissions of parent-dir
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>