This is effectively a revert of 2ac9968401, which
switched from os/exec to the golang.org/x/sys/execabs package to mitigate
security issues (mainly on Windows) with lookups resolving to binaries in the
current directory.
from the go1.19 release notes https://go.dev/doc/go1.19#os-exec-path
> ## PATH lookups
>
> Command and LookPath no longer allow results from a PATH search to be found
> relative to the current directory. This removes a common source of security
> problems but may also break existing programs that depend on using, say,
> exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe) in
> the current directory. See the os/exec package documentation for information
> about how best to update such programs.
>
> On Windows, Command and LookPath now respect the NoDefaultCurrentDirectoryInExePath
> environment variable, making it possible to disable the default implicit search
> of “.” in PATH lookups on Windows systems.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2af6db672e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Fixes ae70213
In ae70213 the WritePidFile and WriteAddress functions were
changed to use AtomicFile instead of os.CreateFile. However,
AtomicFile creates a temporary file and then changes its permissions
with os.Chmod which alters the previously observed behavior of
os.CreateFile which takes the system's umask into account.
This means that on Linux-based systems these files suddenly
became world writable (#9363).
Signed-off-by: Jaroslav Jindrak <dzejrou@gmail.com>
Fixes ae7021300
In ae7021300 the WritePidFile and WriteAddress functions were
changed to use AtomicFile instead of os.CreateFile. However,
AtomicFile creates a temporary file and then changes its permissions
with os.Chmod which alters the previously observed behavior of
os.CreateFile which takes the system's umask into account.
This means that on Linux-based systems these files suddenly
became world writable (#9363). The address file has since been
removed, but pid-file was still created as world writable. This
commit explicitly requests 0644 permissions as even on systems
without default umask of 0022 there is no reason to have these
two files world writable.
Signed-off-by: Jaroslav Jindrak <dzejrou@gmail.com>
(cherry picked from commit 9d328410a5)
Signed-off-by: Jaroslav Jindrak <dzejrou@gmail.com>
This is mostly to workaround an issue with gRPC based shims after containerd
restart. If a shim dies while containerd is also down/restarting, on reboot
grpc.DialContext with our current set of DialOptions will make us wait for
100 seconds per shim even if the socket no longer exists or has no listener.
Signed-off-by: Danny Canter <danny@dcantah.dev>
(cherry picked from commit 0bc9633414)
Signed-off-by: Danny Canter <danny@dcantah.dev>
```go
// Delete the initial process and container
func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteResponse, error) {
p, err := s.getInitProcess()
if err != nil {
return nil, err
}
if err := p.Delete(ctx); err != nil {
return nil, errdefs.ToGRPC(err)
}
// The client might canceled the request but the shim service still
// moved on. The `delete(s.processes, s.id)` was executed
// successfully. So the next Delete call will return `container
// must be created` error. The client side should ignore this
// issue.
s.mu.Lock()
delete(s.processes, s.id)
s.mu.Unlock()
s.platform.Close()
return &shimapi.DeleteResponse{
ExitStatus: uint32(p.ExitStatus()),
ExitedAt: protobuf.ToTimestamp(p.ExitedAt()),
Pid: uint32(p.Pid()),
}, nil
}
```
introduced by #9003fixes: #9309
Signed-off-by: Wei Fu <fuweid89@gmail.com>
After pr #8617, create handler of containerd-shim-runc-v2 will
call handleStarted() to record the init process and handle its exit.
Init process wouldn't quit so early in normal circumstances. But if
this screnario occurs, handleStarted() will call
handleProcessExit(), which will cause deadlock because create() had
acquired s.mu, and handleProcessExit() will try to lock it again.
So, I added a parameter muLocked to handleStarted to indicate whether
or not s.mu is currently locked, and thus deciding whether or not to
lock it when calling handleProcessExit.
Fix: #9103
Signed-off-by: Chen Yiyang <cyyzero@qq.com>
(cherry picked from commit 68dd47ef70)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previous code has already called `getContainer()`, just pass it into
`s.getContainerPids` to reduce unnecessary lock and map lookup.
Signed-off-by: Chen Yiyang <cyyzero@qq.com>
(cherry picked from commit 6604ff6c55)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since the moby/moby can't handle duplicate exit event well, it's hard
for containerd to retry shutdown if there is error, like context
canceled.
In order to prevent from regression like #4769, I add skipped
integration case as TODO item and we should rethink about how to handle
the task/shim lifecycle.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
(cherry picked from commit 601699a184)
Signed-off-by: Wei Fu <fuweid89@gmail.com>
eventSendMu is causing severe lock contention when multiple processes
start and exit concurrently. Replace it with a different scheme for
maintaining causality w.r.t. start and exit events for a process which
does not rely on big locks for synchronization.
Keep track of all processes for which a Task(Exec)Start event has been
published and have not yet exited in a map, keyed by their PID.
Processing exits then is as simple as looking up which process
corresponds to the PID. If there are no started processes known with
that PID, the PID must either belong to a process which was started by
s.Start() and before the s.Start() call has added the process to the map
of running processes, or a reparented process which we don't care about.
Handle the former case by having each s.Start() call subscribe to exit
events before starting the process. It checks if the PID has exited in
the time between it starting the process and publishing the TaskStart
event, handling the exit if it has. Exit events for reparented processes
received when no s.Start() calls are in flight are immediately
discarded, and events received during an s.Start() call are discarded
when the s.Start() call returns.
Co-authored-by: Laura Brehm <laurabrehm@hey.com>
Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 5cd6210ad0)
Signed-off-by: Cory Snider <csnider@mirantis.com>
We pass in a callback using the ttrpc.WithOnClose functionality
for shims that use ttrpc, but with the newly added ability to use
GRPC for shims this was left as a follow-up. It doesn't seem like
grpc-go has anything similar so some options (that I could see) are:
This change introduces a new grpcConn wrapper type for the connection
that exposes a method to get notified when the users callback has run,
the same in functionality as TTRPC's `UserOnCloseWait`. The callback
gets passed in in a new `grpcDialContext` function that will:
1. Dial the connection as normal
2. Spin off a goroutine that will monitor the connections state
until it transitions to idle or shutdown and will then run the
callback.
Signed-off-by: Danny Canter <danny@dcantah.dev>
- When tracing code, it was a bit hard to understand what the third parameter is.
- The current comment should be enough to understand how to use LoggerFunc,
and people who want to learn more can click into the doc link.
Signed-off-by: Hsing-Yu (David) Chen <davidhsingyuchen@gmail.com>
This updates the runtime/v2 README to state the experimental GRPC support
for shims and how to choose the protocol.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Recent work added the ability to use grpc for shims, it'd be nice to
have a debug (or info perhaps) log to show what protocol and addr the
shim sent over.
Signed-off-by: Danny Canter <danny@dcantah.dev>
- Add Target to mount.Mount.
- Add UnmountMounts to unmount a list of mounts in reverse order.
- Add UnmountRecursive to unmount deepest mount first for a given target, using
moby/sys/mountinfo.
Signed-off-by: Edgar Lee <edgarhinshunlee@gmail.com>
Akihiro Suda