Merge pull request #1105 from hashicorp/b-forked

Put the executor into the cgroup to avoid a fork race
This commit is contained in:
Alex Dadgar 2016-04-19 15:54:27 -07:00
commit e464c838d7
5 changed files with 105 additions and 37 deletions

View File

@ -219,7 +219,9 @@ func (d *ExecDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro
merrs.Errors = append(merrs.Errors, fmt.Errorf("error destroying plugin and userpid: %v", e))
}
if id.IsolationConfig != nil {
if e := executor.DestroyCgroup(id.IsolationConfig.Cgroup, id.IsolationConfig.CgroupPaths); e != nil {
isoConf := id.IsolationConfig
ePid := pluginConfig.Reattach.Pid
if e := executor.DestroyCgroup(isoConf.Cgroup, isoConf.CgroupPaths, ePid); e != nil {
merrs.Errors = append(merrs.Errors, fmt.Errorf("destroying cgroup failed: %v", e))
}
}
@ -317,7 +319,9 @@ func (h *execHandle) run() {
// user pid might be holding onto.
if ps.ExitCode == 0 && err != nil {
if h.isolationConfig != nil {
if e := executor.DestroyCgroup(h.isolationConfig.Cgroup, h.isolationConfig.CgroupPaths); e != nil {
isoConf := h.isolationConfig
ePid := h.pluginClient.ReattachConfig().Pid
if e := executor.DestroyCgroup(isoConf.Cgroup, isoConf.CgroupPaths, ePid); e != nil {
h.logger.Printf("[ERR] driver.exec: destroying cgroup failed while killing cgroup: %v", e)
}
}

View File

@ -239,11 +239,15 @@ func (e *UniversalExecutor) LaunchCmd(command *ExecCommand, ctx *ExecutorContext
e.cmd.Args = append([]string{path}, ctx.TaskEnv.ParseAndReplace(command.Args)...)
e.cmd.Env = ctx.TaskEnv.EnvList()
// Start the process
if err := e.cmd.Start(); err != nil {
// Apply ourselves into the cgroup. The executor MUST be in the cgroup
// before the user task is started, otherwise we are subject to a fork
// attack in which a process escapes isolation by immediately forking.
if err := e.applyLimits(os.Getpid()); err != nil {
return nil, err
}
if err := e.applyLimits(e.cmd.Process.Pid); err != nil {
// Start the process
if err := e.cmd.Start(); err != nil {
return nil, err
}
go e.wait()
@ -338,7 +342,10 @@ func (e *UniversalExecutor) wait() {
exitCode = 128 + signal
}
}
} else {
e.logger.Printf("[DEBUG] executor: unexpected Wait() error type: %v", err)
}
e.exitState = &ProcessState{Pid: 0, ExitCode: exitCode, Signal: signal, IsolationConfig: ic, Time: time.Now()}
}
@ -358,7 +365,13 @@ func (e *UniversalExecutor) Exit() error {
e.lre.Close()
e.lro.Close()
if e.command != nil && e.cmd.Process != nil {
// If the executor did not launch a process, return.
if e.command == nil {
return nil
}
// Prefer killing the process via cgroups.
if e.cmd.Process != nil && !e.command.ResourceLimits {
proc, err := os.FindProcess(e.cmd.Process.Pid)
if err != nil {
e.logger.Printf("[ERR] executor: can't find process with pid: %v, err: %v",
@ -369,18 +382,19 @@ func (e *UniversalExecutor) Exit() error {
}
}
if e.command != nil && e.command.FSIsolation {
if err := e.removeChrootMounts(); err != nil {
merr.Errors = append(merr.Errors, err)
}
}
if e.command != nil && e.command.ResourceLimits {
if e.command.ResourceLimits {
e.cgLock.Lock()
if err := DestroyCgroup(e.groups, e.cgPaths); err != nil {
if err := DestroyCgroup(e.groups, e.cgPaths, os.Getpid()); err != nil {
merr.Errors = append(merr.Errors, err)
}
e.cgLock.Unlock()
}
if e.command.FSIsolation {
if err := e.removeChrootMounts(); err != nil {
merr.Errors = append(merr.Errors, err)
}
}
return merr.ErrorOrNil()
}
@ -391,12 +405,15 @@ func (e *UniversalExecutor) ShutDown() error {
}
proc, err := os.FindProcess(e.cmd.Process.Pid)
if err != nil {
return fmt.Errorf("executor.shutdown error: %v", err)
return fmt.Errorf("executor.shutdown failed to find process: %v", err)
}
if runtime.GOOS == "windows" {
return proc.Kill()
if err := proc.Kill(); err != nil && err.Error() != finishedErr {
return err
}
return nil
}
if err = proc.Signal(os.Interrupt); err != nil {
if err = proc.Signal(os.Interrupt); err != nil && err.Error() != finishedErr {
return fmt.Errorf("executor.shutdown error: %v", err)
}
return nil

View File

@ -8,7 +8,7 @@ func (e *UniversalExecutor) configureChroot() error {
return nil
}
func DestroyCgroup(groups *cgroupConfig.Cgroup, paths map[string]string) error {
func DestroyCgroup(groups *cgroupConfig.Cgroup, paths map[string]string, executorPid int) error {
return nil
}

View File

@ -6,6 +6,7 @@ import (
"os/user"
"path/filepath"
"strconv"
"strings"
"syscall"
"github.com/hashicorp/go-multierror"
@ -67,7 +68,7 @@ func (e *UniversalExecutor) applyLimits(pid int) error {
cgConfig := cgroupConfig.Config{Cgroups: e.groups}
if err := manager.Set(&cgConfig); err != nil {
e.logger.Printf("[ERR] executor: error setting cgroup config: %v", err)
if er := DestroyCgroup(e.groups, e.cgPaths); er != nil {
if er := DestroyCgroup(e.groups, e.cgPaths, os.Getpid()); er != nil {
e.logger.Printf("[ERR] executor: error destroying cgroup: %v", er)
}
if er := e.removeChrootMounts(); er != nil {
@ -186,34 +187,76 @@ func (e *UniversalExecutor) removeChrootMounts() error {
}
// destroyCgroup kills all processes in the cgroup and removes the cgroup
// configuration from the host.
func DestroyCgroup(groups *cgroupConfig.Cgroup, cgPaths map[string]string) error {
merrs := new(multierror.Error)
// configuration from the host. This function is idempotent.
func DestroyCgroup(groups *cgroupConfig.Cgroup, cgPaths map[string]string, executorPid int) error {
mErrs := new(multierror.Error)
if groups == nil {
return fmt.Errorf("Can't destroy: cgroup configuration empty")
}
// Move the executor into the global cgroup so that the task specific
// cgroup can be destroyed.
nilGroup := &cgroupConfig.Cgroup{}
nilGroup.Path = "/"
nilGroup.Resources = groups.Resources
nilManager := getCgroupManager(nilGroup, nil)
err := nilManager.Apply(executorPid)
if err != nil && !strings.Contains(err.Error(), "no such process") {
return fmt.Errorf("failed to remove executor pid %d: %v", executorPid, err)
}
// Freeze the Cgroup so that it can not continue to fork/exec.
manager := getCgroupManager(groups, cgPaths)
if pids, perr := manager.GetPids(); perr == nil {
for _, pid := range pids {
proc, err := os.FindProcess(pid)
if err != nil {
merrs.Errors = append(merrs.Errors, fmt.Errorf("error finding process %v: %v", pid, err))
} else {
if e := proc.Kill(); e != nil {
merrs.Errors = append(merrs.Errors, fmt.Errorf("error killing process %v: %v", pid, e))
}
}
err = manager.Freeze(cgroupConfig.Frozen)
if err != nil && !strings.Contains(err.Error(), "no such file or directory") {
return fmt.Errorf("failed to freeze cgroup: %v", err)
}
var procs []*os.Process
pids, err := manager.GetAllPids()
if err != nil {
multierror.Append(mErrs, fmt.Errorf("error getting pids: %v", err))
// Unfreeze the cgroup.
err = manager.Freeze(cgroupConfig.Thawed)
if err != nil && !strings.Contains(err.Error(), "no such file or directory") {
multierror.Append(mErrs, fmt.Errorf("failed to unfreeze cgroup: %v", err))
}
} else {
merrs.Errors = append(merrs.Errors, fmt.Errorf("error getting pids: %v", perr))
return mErrs.ErrorOrNil()
}
// Kill the processes in the cgroup
for _, pid := range pids {
proc, err := os.FindProcess(pid)
if err != nil {
multierror.Append(mErrs, fmt.Errorf("error finding process %v: %v", pid, err))
continue
}
procs = append(procs, proc)
if e := proc.Kill(); e != nil {
multierror.Append(mErrs, fmt.Errorf("error killing process %v: %v", pid, e))
}
}
// Unfreeze the cgroug so we can wait.
err = manager.Freeze(cgroupConfig.Thawed)
if err != nil && !strings.Contains(err.Error(), "no such file or directory") {
multierror.Append(mErrs, fmt.Errorf("failed to unfreeze cgroup: %v", err))
}
// Wait on the killed processes to ensure they are cleaned up.
for _, proc := range procs {
// Don't capture the error because we expect this to fail for
// processes we didn't fork.
proc.Wait()
}
// Remove the cgroup.
if err := manager.Destroy(); err != nil {
multierror.Append(merrs, fmt.Errorf("Failed to delete the cgroup directories: %v", err))
multierror.Append(mErrs, fmt.Errorf("failed to delete the cgroup directories: %v", err))
}
return merrs.ErrorOrNil()
return mErrs.ErrorOrNil()
}
// getCgroupManager returns the correct libcontainer cgroup manager.

View File

@ -286,7 +286,9 @@ func (d *JavaDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro
merrs.Errors = append(merrs.Errors, fmt.Errorf("error destroying plugin and userpid: %v", e))
}
if id.IsolationConfig != nil {
if e := executor.DestroyCgroup(id.IsolationConfig.Cgroup, id.IsolationConfig.CgroupPaths); e != nil {
isoConf := id.IsolationConfig
ePid := pluginConfig.Reattach.Pid
if e := executor.DestroyCgroup(isoConf.Cgroup, isoConf.CgroupPaths, ePid); e != nil {
merrs.Errors = append(merrs.Errors, fmt.Errorf("destroying cgroup failed: %v", e))
}
}
@ -383,7 +385,9 @@ func (h *javaHandle) run() {
close(h.doneCh)
if ps.ExitCode == 0 && err != nil {
if h.isolationConfig != nil {
if e := executor.DestroyCgroup(h.isolationConfig.Cgroup, h.isolationConfig.CgroupPaths); e != nil {
isoConf := h.isolationConfig
ePid := h.pluginClient.ReattachConfig().Pid
if e := executor.DestroyCgroup(isoConf.Cgroup, isoConf.CgroupPaths, ePid); e != nil {
h.logger.Printf("[ERR] driver.java: destroying cgroup failed while killing cgroup: %v", e)
}
} else {