mirror/runc
1
0
Fork 0
mirror of https://github.com/opencontainers/runc.git synced 2026-07-10 21:53:57 +08:00
Code Issues Packages Projects Releases Wiki Activity

libct: initProcess.start: fix sync logic

Browse Source 
The code in this function became quite complicated and not entirely
correct over time. As a result, if an error is returned from parseSync,
it might end up stuck waiting for the child to finish.

1. Let's not wait() for the child twice. We already do it in the
defer statement (call p.terminate()) when we are returning an error.

2. Remove sentResume and sentRun since we do not want to check if
these were sent or not. Instead, introduce and check seenProcReady, as
procReady is always expected from runc init.

3. Eliminate the possibility to wrap nil as an error.

4. Make sure we always call shutdown on the sync socket, and do not let
   shutdown error shadow the ierr.

This fixes the issue of stuck `runc runc` with the optimization patch
(sending procSeccompDone earlier) applied.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin
2023-08-15 18:22:38 -07:00
parent b0c7ce5158
commit c6e7b1a8ec
1 changed files with 7 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libcontainer/process_linux.go
+7 -17
View File
@@ -451,11 +451,8 @@ func (p *initProcess) start() (retErr error) {
if err := p.sendConfig(); err != nil {
return fmt.Errorf("error sending config to init process: %w", err)
}
var (
sentRun bool
sentResume bool
)
var seenProcReady bool
ierr := parseSync(p.messageSockPair.parent, func(sync *syncT) error {
switch sync.Type {
case procSeccomp:
@@ -494,6 +491,7 @@ func (p *initProcess) start() (retErr error) {
return err
}
case procReady:
seenProcReady = true
// set rlimits, this has to be done here because we lose permissions
// to raise the limits once we enter a user-namespace
if err := setupRlimits(p.config.Rlimits, p.pid()); err != nil {
@@ -555,7 +553,6 @@ func (p *initProcess) start() (retErr error) {
if err := writeSync(p.messageSockPair.parent, procRun); err != nil {
return err
}
sentRun = true
case procHooks:
// Setup cgroup before prestart hook, so that the prestart hook could apply cgroup permissions.
if err := p.manager.Set(p.config.Config.Cgroups.Resources); err != nil {
@@ -587,7 +584,6 @@ func (p *initProcess) start() (retErr error) {
if err := writeSync(p.messageSockPair.parent, procResume); err != nil {
return err
}
sentResume = true
default:
return errors.New("invalid JSON payload from child")
}
@@ -595,20 +591,14 @@ func (p *initProcess) start() (retErr error) {
return nil
})
if !sentRun {
return fmt.Errorf("error during container init: %w", ierr)
}
if p.config.Config.Namespaces.Contains(configs.NEWNS) && !sentResume {
return errors.New("could not synchronise after executing prestart and CreateRuntime hooks with container process")
}
if err := unix.Shutdown(int(p.messageSockPair.parent.Fd()), unix.SHUT_WR); err != nil {
if err := unix.Shutdown(int(p.messageSockPair.parent.Fd()), unix.SHUT_WR); err != nil && ierr == nil {
return &os.PathError{Op: "shutdown", Path: "(init pipe)", Err: err}
}
// Must be done after Shutdown so the child will exit and we can wait for it.
if !seenProcReady && ierr == nil {
ierr = errors.New("procReady not received")
}
if ierr != nil {
_, _ = p.wait()
return ierr
return fmt.Errorf("error during container init: %w", ierr)
}
return nil
}