1. docker
  2. Commit 735e25032659e4e46d8e1bf3c9fcc5b5ec785ec2
Browse code

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 <github@gone.nl>

Sebastiaan van Stijn authored on 2022/10/16 06:14:25
Showing 2 changed files
pkg/process/process_test.go
History View file @ 735e250
1 1 
new file mode 100644
... ... 
@@ -0,0 +1,40 @@
0 
+package process
1 
+
2 
+import (
3 
+	"fmt"
4 
+	"os"
5 
+	"os/exec"
6 
+	"runtime"
7 
+	"testing"
8 
+)
9 
+
10 
+func TestAlive(t *testing.T) {
11 
+	for _, pid := range []int{0, -1, -123} {
12 
+		t.Run(fmt.Sprintf("invalid process (%d)", pid), func(t *testing.T) {
13 
+			if Alive(pid) {
14 
+				t.Errorf("PID %d should not be alive", pid)
15 
+			}
16 
+		})
17 
+	}
18 
+	t.Run("current process", func(t *testing.T) {
19 
+		if pid := os.Getpid(); !Alive(pid) {
20 
+			t.Errorf("current PID (%d) should be alive", pid)
21 
+		}
22 
+	})
23 
+	t.Run("exited process", func(t *testing.T) {
24 
+		if runtime.GOOS == "windows" {
25 
+			t.Skip("TODO: make this work on Windows")
26 
+		}
27 
+
28 
+		// Get a PID of an exited process.
29 
+		cmd := exec.Command("echo", "hello world")
30 
+		err := cmd.Run()
31 
+		if err != nil {
32 
+			t.Fatal(err)
33 
+		}
34 
+		exitedPID := cmd.ProcessState.Pid()
35 
+		if Alive(exitedPID) {
36 
+			t.Errorf("PID %d should not be alive", exitedPID)
37 
+		}
38 
+	})
39 
+}
pkg/process/process_unix.go
History View file @ 735e250
... ... 
@@ -14,8 +14,14 @@ import (
14 14 
 	"golang.org/x/sys/unix"
15 15 
 )
16 16
17 
-// Alive returns true if process with a given pid is running.
17 
+// Alive returns true if process with a given pid is running. It only considers
18 
+// positive PIDs; 0 (all processes in the current process group), -1 (all processes
19 
+// with a PID larger than 1), and negative (-n, all processes in process group
20 
+// "n") values for pid are never considered to be alive.
18 21 
 func Alive(pid int) bool {
22 
+	if pid < 1 {
23 
+		return false
24 
+	}
19 25 
 	switch runtime.GOOS {
20 26 
 	case "darwin":
21 27 
 		// 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 {
35 35 
 	}
36 36 
 }
37 37
38 
-// Kill force-stops a process.
38 
+// Kill force-stops a process. It only considers positive PIDs; 0 (all processes
39 
+// in the current process group), -1 (all processes with a PID larger than 1),
40 
+// and negative (-n, all processes in process group "n") values for pid are
41 
+// ignored. Refer to [KILL(2)] for details.
42 
+//
43 
+// [KILL(2)]: https://man7.org/linux/man-pages/man2/kill.2.html
39 44 
 func Kill(pid int) error {
45 
+	if pid < 1 {
46 
+		return fmt.Errorf("invalid PID (%d): only positive PIDs are allowed", pid)
47 
+	}
40 48 
 	err := unix.Kill(pid, unix.SIGKILL)
41 49 
 	if err != nil && err != unix.ESRCH {
42 50 
 		return err
... ... 
@@ -44,9 +58,16 @@ func Kill(pid int) error {
44 44 
 	return nil
45 45 
 }
46 46
47 
-// Zombie return true if process has a state with "Z"
48 
-// http://man7.org/linux/man-pages/man5/proc.5.html
47 
+// Zombie return true if process has a state with "Z". It only considers positive
48 
+// PIDs; 0 (all processes in the current process group), -1 (all processes with
49 
+// a PID larger than 1), and negative (-n, all processes in process group "n")
50 
+// values for pid are ignored. Refer to [PROC(5)] for details.
51 
+//
52 
+// [PROC(5)]: https://man7.org/linux/man-pages/man5/proc.5.html
49 53 
 func Zombie(pid int) (bool, error) {
54 
+	if pid < 1 {
55 
+		return false, nil
56 
+	}
50 57 
 	data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid))
51 58 
 	if err != nil {
52 59 
 		if os.IsNotExist(err) {