rootlessnetns: handle empty pid file gracefully

When pasta fails to start, it can leave an empty PID file which causes a crash in readPidFile. This fix handles empty PID files gracefully by returning a specific error and ignoring it during cleanup.

Fixes: containers/podman#28441

Signed-off-by: Alessio Attilio <attilio.alessio@protonmail.com>

Author: kairosci

common/libnetwork/internal/rootlessnetns/netns_linux.go

@@ -45,14 +45,16 @@ const (
 	resolvConfName = "resolv.conf"
 )
 
+var errEmptyPIDFile = errors.New("PID file is empty")
+
 type Netns struct {
 	// dir used for the rootless netns
 	dir string
 
 	// config contains containers.conf options.
 	config *config.Config
 
-	// info contain information about ip addresses used in the netns.
+	// info contains information about ip addresses used in the netns.
 	// A caller can get this info via Info().
 	info *types.RootlessNetnsInfo
 }
@@ -172,7 +174,7 @@ func (n *Netns) getOrCreateNetns() (netns.NetNS, bool, error) {
 func (n *Netns) cleanup() error {
 	if err := fileutils.Exists(n.dir); err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
-			// dir does not exists no need for cleanup
+			// dir does not exist no need for cleanup
 			return nil
 		}
 		return err
@@ -310,18 +312,26 @@ func (n *Netns) setupSlirp4netns(nsPath string) error {
 
 func (n *Netns) cleanupRootlessNetns() error {
 	pidFile := n.getPath(rootlessNetNsConnPidFile)
+
 	pid, err := readPidFile(pidFile)
-	// do not hard error if the file dos not exists, cleanup should be idempotent
+	// do not hard error if the file does not exist, cleanup should be idempotent
 	if errors.Is(err, fs.ErrNotExist) {
-		logrus.Debugf("Rootless netns conn pid file does not exists %s", pidFile)
+		logrus.Debugf("Rootless netns conn pid file does not exist %s", pidFile)
 		return nil
 	}
-	if err == nil {
-		// kill the slirp/pasta process so we do not leak it
-		err = unix.Kill(pid, unix.SIGTERM)
-		if err == unix.ESRCH {
-			err = nil
-		}
+	// if the pid file is empty, pasta failed to start so there is nothing to clean up
+	if errors.Is(err, errEmptyPIDFile) {
+		logrus.Debugf("Rootless netns conn pid file is empty, nothing to clean up")
+		return nil
+	}
+	if err != nil {
+		logrus.Warnf("Rootless netns conn pid file is invalid: %v", err)
+		return nil
+	}
+	// kill the slirp/pasta process so we do not leak it
+	err = unix.Kill(pid, unix.SIGTERM)
+	if err == unix.ESRCH {
+		err = nil
 	}
 	return err
 }
@@ -602,9 +612,12 @@ func (n *Netns) Run(lock *lockfile.LockFile, toRun func() error) error {
 	}
 
 	if count == 0 {
-		err = n.cleanup()
-		if err != nil {
-			return wrapError("cleanup", err)
+		cleanupErr := n.cleanup()
+		if cleanupErr != nil {
+			if inErr != nil {
+				return wrapError("cleanup", fmt.Errorf("%v: %w", inErr, cleanupErr))
+			}
+			return wrapError("cleanup", cleanupErr)
 		}
 	}
 
@@ -652,7 +665,11 @@ func readPidFile(path string) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	return strconv.Atoi(strings.TrimSpace(string(b)))
+	s := strings.TrimSpace(string(b))
+	if s == "" {
+		return 0, errEmptyPIDFile
+	}
+	return strconv.Atoi(s)
 }
 
 func (n *Netns) serializeInfo() error {

common/libnetwork/internal/rootlessnetns/netns_linux_test.go

@@ -75,3 +75,66 @@ func Test_refCount(t *testing.T) {
 		})
 	}
 }
+
+func TestReadPidFile(t *testing.T) {
+	tests := []struct {
+		name    string
+		content string
+		create  bool
+		wantErr error
+		wantPid int
+	}{
+		{
+			name:    "valid pid",
+			content: "1234\n",
+			create:  true,
+			wantPid: 1234,
+		},
+		{
+			name:    "valid pid no newline",
+			content: "5678",
+			create:  true,
+			wantPid: 5678,
+		},
+		{
+			name:    "empty pid file",
+			content: "",
+			create:  true,
+			wantErr: errEmptyPIDFile,
+		},
+		{
+			name:    "only whitespace",
+			content: "  \n ",
+			create:  true,
+			wantErr: errEmptyPIDFile,
+		},
+		{
+			name:    "non-existent file",
+			create:  false,
+			wantErr: os.ErrNotExist,
+		},
+		{
+			name:    "invalid content",
+			content: "abc",
+			create:  true,
+			wantErr: strconv.ErrSyntax,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			path := filepath.Join(t.TempDir(), "pidfile")
+			if tt.create {
+				err := os.WriteFile(path, []byte(tt.content), 0o600)
+				assert.NoError(t, err)
+			}
+			pid, err := readPidFile(path)
+			if tt.wantErr != nil {
+				assert.Error(t, err)
+				assert.ErrorIs(t, err, tt.wantErr)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.wantPid, pid)
+			}
+		})
+	}
+}