Skip to content

Commit a41366e

Browse files
committed
openat2: improve resilience on busy systems
Previously, we would see a ~3% failure rate when starting containers with mounts that contain ".." (which can trigger -EAGAIN). To counteract this, filepath-securejoin v0.5.1 includes a bump of the internal retry limit from 32 to 128, which lowers the failure rate to 0.12%. However, there is still a risk of spurious failure on regular systems. In order to try to provide more resilience (while avoiding DoS attacks), this patch also includes an additional retry loop that terminates based on a deadline rather than retry count. The deadline is 2ms, as my testing found that ~800us for a single pathrs operation was the longest latency due to -EAGAIN retries, and that was an outlier compared to the more common ~400us latencies -- so 2ms should be more than enough for any real system. The failure rates above were based on more 50k runs of runc with an attack script (from libpathrs) running a rename attack on all cores of a 16-core system, which is arguably a worst-case but heavily utilised servers could likely approach similar results. Tested-by: Phil Estes <[email protected]> Signed-off-by: Aleksa Sarai <[email protected]>
1 parent ed6b169 commit a41366e

File tree

11 files changed

+144
-26
lines changed

11 files changed

+144
-26
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ require (
66
github.com/checkpoint-restore/go-criu/v7 v7.2.0
77
github.com/containerd/console v1.0.5
88
github.com/coreos/go-systemd/v22 v22.6.0
9-
github.com/cyphar/filepath-securejoin v0.5.0
9+
github.com/cyphar/filepath-securejoin v0.5.1
1010
github.com/docker/go-units v0.5.0
1111
github.com/godbus/dbus/v5 v5.1.0
1212
github.com/moby/sys/capability v0.4.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ github.com/coreos/go-systemd/v22 v22.6.0 h1:aGVa/v8B7hpb0TKl0MWoAavPDmHvobFe5R5z
99
github.com/coreos/go-systemd/v22 v22.6.0/go.mod h1:iG+pp635Fo7ZmV/j14KUcmEyWF+0X7Lua8rrTWzYgWU=
1010
github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo=
1111
github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
12-
github.com/cyphar/filepath-securejoin v0.5.0 h1:hIAhkRBMQ8nIeuVwcAoymp7MY4oherZdAxD+m0u9zaw=
13-
github.com/cyphar/filepath-securejoin v0.5.0/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI=
12+
github.com/cyphar/filepath-securejoin v0.5.1 h1:eYgfMq5yryL4fbWfkLpFFy2ukSELzaJOTaUTuh+oF48=
13+
github.com/cyphar/filepath-securejoin v0.5.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI=
1414
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
1515
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
1616
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

internal/pathrs/mkdirall_pathrslite.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (*os.File, er
8383
}
8484
defer rootDir.Close()
8585

86-
return pathrs.MkdirAllHandle(rootDir, unsafePath, mode)
86+
return retryEAGAIN(func() (*os.File, error) {
87+
return pathrs.MkdirAllHandle(rootDir, unsafePath, mode)
88+
})
8789
}
8890

8991
// MkdirAllInRoot is a wrapper around MkdirAllInRootOpen which closes the

internal/pathrs/procfs_pathrslite.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@ import (
2727
)
2828

2929
func procOpenReopen(openFn func(subpath string) (*os.File, error), subpath string, flags int) (*os.File, error) {
30-
handle, err := openFn(subpath)
30+
handle, err := retryEAGAIN(func() (*os.File, error) {
31+
return openFn(subpath)
32+
})
3133
if err != nil {
3234
return nil, err
3335
}
3436
defer handle.Close()
3537

36-
f, err := pathrs.Reopen(handle, flags)
38+
f, err := Reopen(handle, flags)
3739
if err != nil {
3840
return nil, fmt.Errorf("reopen %s: %w", handle.Name(), err)
3941
}
@@ -44,7 +46,7 @@ func procOpenReopen(openFn func(subpath string) (*os.File, error), subpath strin
4446
// [pathrs.Reopen], to let you one-shot open a procfs file with the given
4547
// flags.
4648
func ProcSelfOpen(subpath string, flags int) (*os.File, error) {
47-
proc, err := procfs.OpenProcRoot()
49+
proc, err := retryEAGAIN(procfs.OpenProcRoot)
4850
if err != nil {
4951
return nil, err
5052
}
@@ -55,7 +57,7 @@ func ProcSelfOpen(subpath string, flags int) (*os.File, error) {
5557
// ProcPidOpen is a wrapper around [procfs.Handle.OpenPid] and [pathrs.Reopen],
5658
// to let you one-shot open a procfs file with the given flags.
5759
func ProcPidOpen(pid int, subpath string, flags int) (*os.File, error) {
58-
proc, err := procfs.OpenProcRoot()
60+
proc, err := retryEAGAIN(procfs.OpenProcRoot)
5961
if err != nil {
6062
return nil, err
6163
}
@@ -70,13 +72,15 @@ func ProcPidOpen(pid int, subpath string, flags int) (*os.File, error) {
7072
// flags. The returned [procfs.ProcThreadSelfCloser] needs the same handling as
7173
// when using pathrs-lite.
7274
func ProcThreadSelfOpen(subpath string, flags int) (_ *os.File, _ procfs.ProcThreadSelfCloser, Err error) {
73-
proc, err := procfs.OpenProcRoot()
75+
proc, err := retryEAGAIN(procfs.OpenProcRoot)
7476
if err != nil {
7577
return nil, nil, err
7678
}
7779
defer proc.Close()
7880

79-
handle, closer, err := proc.OpenThreadSelf(subpath)
81+
handle, closer, err := retryEAGAIN2(func() (*os.File, procfs.ProcThreadSelfCloser, error) {
82+
return proc.OpenThreadSelf(subpath)
83+
})
8084
if err != nil {
8185
return nil, nil, err
8286
}
@@ -89,7 +93,7 @@ func ProcThreadSelfOpen(subpath string, flags int) (_ *os.File, _ procfs.ProcThr
8993
}
9094
defer handle.Close()
9195

92-
f, err := pathrs.Reopen(handle, flags)
96+
f, err := Reopen(handle, flags)
9397
if err != nil {
9498
return nil, nil, fmt.Errorf("reopen %s: %w", handle.Name(), err)
9599
}
@@ -98,5 +102,7 @@ func ProcThreadSelfOpen(subpath string, flags int) (_ *os.File, _ procfs.ProcThr
98102

99103
// Reopen is a wrapper around pathrs.Reopen.
100104
func Reopen(file *os.File, flags int) (*os.File, error) {
101-
return pathrs.Reopen(file, flags)
105+
return retryEAGAIN(func() (*os.File, error) {
106+
return pathrs.Reopen(file, flags)
107+
})
102108
}

internal/pathrs/retry.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2024-2025 Aleksa Sarai <[email protected]>
4+
* Copyright (C) 2024-2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package pathrs
20+
21+
import (
22+
"errors"
23+
"fmt"
24+
"time"
25+
26+
"golang.org/x/sys/unix"
27+
)
28+
29+
// Based on >50k tests running "runc run" on a 16-core system with very heavy
30+
// rename(2) load, the single longest latency caused by -EAGAIN retries was
31+
// ~800us (with the vast majority being closer to 400us). So, a 2ms limit
32+
// should give more than enough headroom for any real system in practice.
33+
const retryDeadline = 2 * time.Millisecond
34+
35+
// retryEAGAIN is a top-level retry loop for pathrs to try to returning
36+
// spurious errors in most normal user cases when using openat2 (libpathrs
37+
// itself does up to 128 retries already, but this method takes a
38+
// wallclock-deadline approach to simply retry until a timer elapses).
39+
func retryEAGAIN[T any](fn func() (T, error)) (T, error) {
40+
deadline := time.After(retryDeadline)
41+
for {
42+
v, err := fn()
43+
if !errors.Is(err, unix.EAGAIN) {
44+
return v, err
45+
}
46+
select {
47+
case <-deadline:
48+
return *new(T), fmt.Errorf("%v retry deadline exceeded: %w", retryDeadline, err)
49+
default:
50+
// retry
51+
}
52+
}
53+
}
54+
55+
// retryEAGAIN2 is like retryEAGAIN except it returns two values.
56+
func retryEAGAIN2[T1, T2 any](fn func() (T1, T2, error)) (T1, T2, error) {
57+
type ret struct {
58+
v1 T1
59+
v2 T2
60+
}
61+
v, err := retryEAGAIN(func() (ret, error) {
62+
v1, v2, err := fn()
63+
return ret{v1: v1, v2: v2}, err
64+
})
65+
return v.v1, v.v2, err
66+
}

internal/pathrs/root_pathrslite.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,15 @@ import (
3333
// is effectively shorthand for [securejoin.OpenInRoot] followed by
3434
// [securejoin.Reopen].
3535
func OpenInRoot(root, subpath string, flags int) (*os.File, error) {
36-
handle, err := pathrs.OpenInRoot(root, subpath)
36+
handle, err := retryEAGAIN(func() (*os.File, error) {
37+
return pathrs.OpenInRoot(root, subpath)
38+
})
3739
if err != nil {
3840
return nil, err
3941
}
4042
defer handle.Close()
41-
return pathrs.Reopen(handle, flags)
43+
44+
return Reopen(handle, flags)
4245
}
4346

4447
// CreateInRoot creates a new file inside a root (as well as any missing parent

vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md

Lines changed: 32 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/github.com/cyphar/filepath-securejoin/VERSION

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/errors.go renamed to vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/errors_linux.go

Lines changed: 13 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/openat2_linux.go

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)