From 735e25032659e4e46d8e1bf3c9fcc5b5ec785ec2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 15 Oct 2022 23:14:25 +0200 Subject: [PATCH] pkg/process: Alive(): fix PID 0, -1, negative values unix.Kill() does not produce an error for PID 0, -1. As a result, checking process.Alive() would return "true" for both 0 and -1 on macOS (and previously on Linux as well). Let's shortcut these values to consider them "not alive", to prevent someone trying to kill them. A basic test was added to check the behavior. Given that the intent of these functions is to handle single processes, this patch also prevents 0 and negative values to be used. From KILL(2): https://man7.org/linux/man-pages/man2/kill.2.html If pid is positive, then signal sig is sent to the process with the ID specified by pid. If pid equals 0, then sig is sent to every process in the process group of the calling process. If pid equals -1, then sig is sent to every process for which the calling process has permission to send signals, except for process 1 (init), but see below. If pid is less than -1, then sig is sent to every process in the process group whose ID is -pid. Signed-off-by: Sebastiaan van Stijn --- pkg/process/process_test.go | 40 +++++++++++++++++++++++++++++++++++++ pkg/process/process_unix.go | 29 +++++++++++++++++++++++---- 2 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 pkg/process/process_test.go diff --git a/pkg/process/process_test.go b/pkg/process/process_test.go new file mode 100644 index 0000000000..496f5475d9 --- /dev/null +++ b/pkg/process/process_test.go @@ -0,0 +1,40 @@ +package process + +import ( + "fmt" + "os" + "os/exec" + "runtime" + "testing" +) + +func TestAlive(t *testing.T) { + for _, pid := range []int{0, -1, -123} { + t.Run(fmt.Sprintf("invalid process (%d)", pid), func(t *testing.T) { + if Alive(pid) { + t.Errorf("PID %d should not be alive", pid) + } + }) + } + t.Run("current process", func(t *testing.T) { + if pid := os.Getpid(); !Alive(pid) { + t.Errorf("current PID (%d) should be alive", pid) + } + }) + t.Run("exited process", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TODO: make this work on Windows") + } + + // Get a PID of an exited process. + cmd := exec.Command("echo", "hello world") + err := cmd.Run() + if err != nil { + t.Fatal(err) + } + exitedPID := cmd.ProcessState.Pid() + if Alive(exitedPID) { + t.Errorf("PID %d should not be alive", exitedPID) + } + }) +} diff --git a/pkg/process/process_unix.go b/pkg/process/process_unix.go index 6057434fa5..daf3923626 100644 --- a/pkg/process/process_unix.go +++ b/pkg/process/process_unix.go @@ -14,8 +14,14 @@ import ( "golang.org/x/sys/unix" ) -// Alive returns true if process with a given pid is running. +// Alive returns true if process with a given pid is running. It only considers +// positive PIDs; 0 (all processes in the current process group), -1 (all processes +// with a PID larger than 1), and negative (-n, all processes in process group +// "n") values for pid are never considered to be alive. func Alive(pid int) bool { + if pid < 1 { + return false + } switch runtime.GOOS { case "darwin": // OS X does not have a proc filesystem. Use kill -0 pid to judge if the @@ -35,8 +41,16 @@ func Alive(pid int) bool { } } -// Kill force-stops a process. +// Kill force-stops a process. It only considers positive PIDs; 0 (all processes +// in the current process group), -1 (all processes with a PID larger than 1), +// and negative (-n, all processes in process group "n") values for pid are +// ignored. Refer to [KILL(2)] for details. +// +// [KILL(2)]: https://man7.org/linux/man-pages/man2/kill.2.html func Kill(pid int) error { + if pid < 1 { + return fmt.Errorf("invalid PID (%d): only positive PIDs are allowed", pid) + } err := unix.Kill(pid, unix.SIGKILL) if err != nil && err != unix.ESRCH { return err @@ -44,9 +58,16 @@ func Kill(pid int) error { return nil } -// Zombie return true if process has a state with "Z" -// http://man7.org/linux/man-pages/man5/proc.5.html +// Zombie return true if process has a state with "Z". It only considers positive +// PIDs; 0 (all processes in the current process group), -1 (all processes with +// a PID larger than 1), and negative (-n, all processes in process group "n") +// values for pid are ignored. Refer to [PROC(5)] for details. +// +// [PROC(5)]: https://man7.org/linux/man-pages/man5/proc.5.html func Zombie(pid int) (bool, error) { + if pid < 1 { + return false, nil + } data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid)) if err != nil { if os.IsNotExist(err) {