Put the executor into the cgroup to avoid a fork race

This commit is contained in:
Alex Dadgar
2016-04-18 17:20:11 -07:00
parent c3c3b66fbf
commit 7616be8094
5 changed files with 46 additions and 14 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()
@@ -376,7 +380,7 @@ func (e *UniversalExecutor) Exit() error {
}
if e.command != nil && 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()

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

@@ -67,7 +67,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 {
@@ -187,15 +187,21 @@ 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 {
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")
}
manager := getCgroupManager(groups, cgPaths)
if pids, perr := manager.GetPids(); perr == nil {
if pids, perr := manager.GetAllPids(); perr == nil {
for _, pid := range pids {
// If the pid is the pid of the executor then we don't kill it, the
// executor is going to be killed by the driver once the Wait
// returns
if pid == executorPid {
continue
}
proc, err := os.FindProcess(pid)
if err != nil {
merrs.Errors = append(merrs.Errors, fmt.Errorf("error finding process %v: %v", pid, err))
@@ -203,15 +209,29 @@ func DestroyCgroup(groups *cgroupConfig.Cgroup, cgPaths map[string]string) error
if e := proc.Kill(); e != nil {
merrs.Errors = append(merrs.Errors, fmt.Errorf("error killing process %v: %v", pid, e))
}
// Don't capture the error because we expect this to fail for
// processes we didn't fork.
proc.Wait()
}
}
} else {
merrs.Errors = append(merrs.Errors, fmt.Errorf("error getting pids: %v", perr))
}
// 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)
if err := nilManager.Apply(executorPid); err != nil {
multierror.Append(merrs, fmt.Errorf("failed to remove executor pid %d: %v", executorPid, err))
}
// 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()
}

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 {