Skip to content

Commit a922767

Browse files
committed
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>
1 parent 638a7b8 commit a922767

File tree

2 files changed

+88
-8
lines changed

2 files changed

+88
-8
lines changed

common/libnetwork/internal/rootlessnetns/netns_linux.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -308,20 +308,33 @@ func (n *Netns) setupSlirp4netns(nsPath string) error {
308308
return nil
309309
}
310310

311+
var (
312+
// ErrEmptyPIDFile is returned when the PID file is empty.
313+
ErrEmptyPIDFile = errors.New("PID file is empty")
314+
)
315+
311316
func (n *Netns) cleanupRootlessNetns() error {
312317
pidFile := n.getPath(rootlessNetNsConnPidFile)
318+
313319
pid, err := readPidFile(pidFile)
314-
// do not hard error if the file dos not exists, cleanup should be idempotent
320+
// do not hard error if the file does not exist, cleanup should be idempotent
315321
if errors.Is(err, fs.ErrNotExist) {
316322
logrus.Debugf("Rootless netns conn pid file does not exists %s", pidFile)
317323
return nil
318324
}
319-
if err == nil {
320-
// kill the slirp/pasta process so we do not leak it
321-
err = unix.Kill(pid, unix.SIGTERM)
322-
if err == unix.ESRCH {
323-
err = nil
324-
}
325+
// if the pid file is empty, pasta failed to start so there is nothing to clean up
326+
if errors.Is(err, ErrEmptyPIDFile) {
327+
logrus.Debugf("Rootless netns conn pid file is empty, nothing to clean up")
328+
return nil
329+
}
330+
if err != nil {
331+
logrus.Warnf("Rootless netns conn pid file is invalid: %v", err)
332+
return nil
333+
}
334+
// kill the slirp/pasta process so we do not leak it
335+
err = unix.Kill(pid, unix.SIGTERM)
336+
if err == unix.ESRCH {
337+
err = nil
325338
}
326339
return err
327340
}
@@ -652,7 +665,11 @@ func readPidFile(path string) (int, error) {
652665
if err != nil {
653666
return 0, err
654667
}
655-
return strconv.Atoi(strings.TrimSpace(string(b)))
668+
s := strings.TrimSpace(string(b))
669+
if s == "" {
670+
return 0, ErrEmptyPIDFile
671+
}
672+
return strconv.Atoi(s)
656673
}
657674

658675
func (n *Netns) serializeInfo() error {

common/libnetwork/internal/rootlessnetns/netns_linux_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,66 @@ func Test_refCount(t *testing.T) {
7575
})
7676
}
7777
}
78+
79+
func TestReadPidFile(t *testing.T) {
80+
tests := []struct {
81+
name string
82+
content string
83+
create bool
84+
wantErr error
85+
wantPid int
86+
}{
87+
{
88+
name: "valid pid",
89+
content: "1234\n",
90+
create: true,
91+
wantPid: 1234,
92+
},
93+
{
94+
name: "valid pid no newline",
95+
content: "5678",
96+
create: true,
97+
wantPid: 5678,
98+
},
99+
{
100+
name: "empty pid file",
101+
content: "",
102+
create: true,
103+
wantErr: ErrEmptyPIDFile,
104+
},
105+
{
106+
name: "only whitespace",
107+
content: " \n ",
108+
create: true,
109+
wantErr: ErrEmptyPIDFile,
110+
},
111+
{
112+
name: "non-existent file",
113+
create: false,
114+
wantErr: os.ErrNotExist,
115+
},
116+
{
117+
name: "invalid content",
118+
content: "abc",
119+
create: true,
120+
wantErr: strconv.ErrSyntax,
121+
},
122+
}
123+
for _, tt := range tests {
124+
t.Run(tt.name, func(t *testing.T) {
125+
path := filepath.Join(t.TempDir(), "pidfile")
126+
if tt.create {
127+
err := os.WriteFile(path, []byte(tt.content), 0o600)
128+
assert.NoError(t, err)
129+
}
130+
pid, err := readPidFile(path)
131+
if tt.wantErr != nil {
132+
assert.Error(t, err)
133+
assert.ErrorIs(t, err, tt.wantErr)
134+
} else {
135+
assert.NoError(t, err)
136+
assert.Equal(t, tt.wantPid, pid)
137+
}
138+
})
139+
}
140+
}

0 commit comments

Comments
 (0)