Repository: opencontainers/runc
File: libcontainer/container_linux.go (Go)

Code before:
	parent, err := c.newParentProcess(process)
	if err != nil {
		return fmt.Errorf("unable to create new parent process: %w", err)
	}
	// We do not need the cloned binaries once the process is spawned.
	defer process.closeClonedExes()

	// Before starting "runc init", mark all non-stdio open files as O_CLOEXEC
	// to make sure we don't leak any files into "runc init". Any files to be
	// passed to "runc init" through ExtraFiles will get dup2'd by the Go
	// runtime and thus their O_CLOEXEC flag will be cleared. This is some
	// additional protection against attacks like CVE-2024-21626, by making
	// sure we never leak files to "runc init" we didn't intend to.
	if err := utils.CloseExecFrom(3); err != nil {
		return fmt.Errorf("unable to mark non-stdio fds as cloexec: %w", err)
	}

	if logsDone := parent.forwardChildLogs(); logsDone != nil {
		defer func() {
			// Wait for log forwarder to finish. This depends on
			// runc init closing the _LIBCONTAINER_LOGPIPE log fd.
			err := <-logsDone
			// An error from child logs is nsexec FATAL messages.
			// Always append those to the original error.
			if err != nil {
				retErr = fmt.Errorf("%w; runc init error(s): %w ", retErr, err)
			}
		}()
	}

	if err := parent.start(); err != nil {
		return fmt.Errorf("unable to start container process: %w", err)
	}

	if process.Init {
		c.fifo.Close()
		if c.config.HasHook(configs.Poststart) {
			s, err := c.currentOCIState()
			if err != nil {
				return err
			}

			if err := c.config.Hooks.Run(configs.Poststart, s); err != nil {
				if err := ignoreTerminateErrors(parent.terminate()); err != nil {
					logrus.Warn(fmt.Errorf("error running poststart hook: %w", err))
				}
				return err
			}
		}
	}
	return nil

Review comment (bug):
This doesn't handle the `retErr == nil` case anymore, which will now result in `%!w(<nil>)`. Maybe this should instead be something more like:

```go
err := <-logsDone
if err != nil {
  err = fmt.Errorf("runc init error(s): %w", err)
  if retErr != nil {
    retErr = fmt.Errorf("%w; %w", retErr, err)
  } else {
    retErr = err
  }
}
```

Also, is the trailing space in the error message string intentional?

Code after:
	parent, err := c.newParentProcess(process)
	if err != nil {
		return fmt.Errorf("unable to create new parent process: %w", err)
	}
	// We do not need the cloned binaries once the process is spawned.
	defer process.closeClonedExes()

	// Before starting "runc init", mark all non-stdio open files as O_CLOEXEC
	// to make sure we don't leak any files into "runc init". Any files to be
	// passed to "runc init" through ExtraFiles will get dup2'd by the Go
	// runtime and thus their O_CLOEXEC flag will be cleared. This is some
	// additional protection against attacks like CVE-2024-21626, by making
	// sure we never leak files to "runc init" we didn't intend to.
	if err := utils.CloseExecFrom(3); err != nil {
		return fmt.Errorf("unable to mark non-stdio fds as cloexec: %w", err)
	}

	if logsDone := parent.forwardChildLogs(); logsDone != nil {
		defer func() {
			// Wait for log forwarder to finish. This depends on
			// runc init closing the _LIBCONTAINER_LOGPIPE log fd.
			err := <-logsDone
			if err != nil {
				// runc init errors are important; make sure retErr has them.
				err = fmt.Errorf("runc init error(s): %w", err)
				if retErr != nil {
					retErr = fmt.Errorf("%w; %w", retErr, err)
				} else {
					retErr = err
				}
			}
		}()
	}

	if err := parent.start(); err != nil {
		return fmt.Errorf("unable to start container process: %w", err)
	}

	if process.Init {
		c.fifo.Close()
		if c.config.HasHook(configs.Poststart) {
			s, err := c.currentOCIState()
			if err != nil {
				return err
			}

			if err := c.config.Hooks.Run(configs.Poststart, s); err != nil {
				if err := ignoreTerminateErrors(parent.terminate()); err != nil {
					logrus.Warn(fmt.Errorf("error running poststart hook: %w", err))
				}
				return err