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]>

Author: cyphar

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/cilium/ebpf v0.16.0
 	github.com/containerd/console v1.0.5
 	github.com/coreos/go-systemd/v22 v22.5.0
-	github.com/cyphar/filepath-securejoin v0.5.0
+	github.com/cyphar/filepath-securejoin v0.5.1
 	github.com/docker/go-units v0.5.0
 	github.com/godbus/dbus/v5 v5.1.0
 	github.com/moby/sys/mountinfo v0.7.1

go.sum

@@ -9,8 +9,8 @@ github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=
 github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
-github.com/cyphar/filepath-securejoin v0.5.0 h1:hIAhkRBMQ8nIeuVwcAoymp7MY4oherZdAxD+m0u9zaw=
-github.com/cyphar/filepath-securejoin v0.5.0/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI=
+github.com/cyphar/filepath-securejoin v0.5.1 h1:eYgfMq5yryL4fbWfkLpFFy2ukSELzaJOTaUTuh+oF48=
+github.com/cyphar/filepath-securejoin v0.5.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

internal/pathrs/mkdirall_pathrslite.go

@@ -83,7 +83,9 @@ func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (*os.File, er
 	}
 	defer rootDir.Close()
 
-	return pathrs.MkdirAllHandle(rootDir, unsafePath, mode)
+	return retryEAGAIN(func() (*os.File, error) {
+		return pathrs.MkdirAllHandle(rootDir, unsafePath, mode)
+	})
 }
 
 // MkdirAllInRoot is a wrapper around MkdirAllInRootOpen which closes the

internal/pathrs/procfs_pathrslite.go

@@ -27,13 +27,15 @@ import (
 )
 
 func procOpenReopen(openFn func(subpath string) (*os.File, error), subpath string, flags int) (*os.File, error) {
-	handle, err := openFn(subpath)
+	handle, err := retryEAGAIN(func() (*os.File, error) {
+		return openFn(subpath)
+	})
 	if err != nil {
 		return nil, err
 	}
 	defer handle.Close()
 
-	f, err := pathrs.Reopen(handle, flags)
+	f, err := Reopen(handle, flags)
 	if err != nil {
 		return nil, fmt.Errorf("reopen %s: %w", handle.Name(), err)
 	}
@@ -44,7 +46,7 @@ func procOpenReopen(openFn func(subpath string) (*os.File, error), subpath strin
 // [pathrs.Reopen], to let you one-shot open a procfs file with the given
 // flags.
 func ProcSelfOpen(subpath string, flags int) (*os.File, error) {
-	proc, err := procfs.OpenProcRoot()
+	proc, err := retryEAGAIN(procfs.OpenProcRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -55,7 +57,7 @@ func ProcSelfOpen(subpath string, flags int) (*os.File, error) {
 // ProcPidOpen is a wrapper around [procfs.Handle.OpenPid] and [pathrs.Reopen],
 // to let you one-shot open a procfs file with the given flags.
 func ProcPidOpen(pid int, subpath string, flags int) (*os.File, error) {
-	proc, err := procfs.OpenProcRoot()
+	proc, err := retryEAGAIN(procfs.OpenProcRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -70,13 +72,15 @@ func ProcPidOpen(pid int, subpath string, flags int) (*os.File, error) {
 // flags. The returned [procfs.ProcThreadSelfCloser] needs the same handling as
 // when using pathrs-lite.
 func ProcThreadSelfOpen(subpath string, flags int) (_ *os.File, _ procfs.ProcThreadSelfCloser, Err error) {
-	proc, err := procfs.OpenProcRoot()
+	proc, err := retryEAGAIN(procfs.OpenProcRoot)
 	if err != nil {
 		return nil, nil, err
 	}
 	defer proc.Close()
 
-	handle, closer, err := proc.OpenThreadSelf(subpath)
+	handle, closer, err := retryEAGAIN2(func() (*os.File, procfs.ProcThreadSelfCloser, error) {
+		return proc.OpenThreadSelf(subpath)
+	})
 	if err != nil {
 		return nil, nil, err
 	}
@@ -89,7 +93,7 @@ func ProcThreadSelfOpen(subpath string, flags int) (_ *os.File, _ procfs.ProcThr
 	}
 	defer handle.Close()
 
-	f, err := pathrs.Reopen(handle, flags)
+	f, err := Reopen(handle, flags)
 	if err != nil {
 		return nil, nil, fmt.Errorf("reopen %s: %w", handle.Name(), err)
 	}
@@ -98,5 +102,7 @@ func ProcThreadSelfOpen(subpath string, flags int) (_ *os.File, _ procfs.ProcThr
 
 // Reopen is a wrapper around pathrs.Reopen.
 func Reopen(file *os.File, flags int) (*os.File, error) {
-	return pathrs.Reopen(file, flags)
+	return retryEAGAIN(func() (*os.File, error) {
+		return pathrs.Reopen(file, flags)
+	})
 }

internal/pathrs/retry.go

@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: Apache-2.0
+/*
+ * Copyright (C) 2024-2025 Aleksa Sarai <[email protected]>
+ * Copyright (C) 2024-2025 SUSE LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package pathrs
+
+import (
+	"errors"
+	"fmt"
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+// Based on >50k tests running "runc run" on a 16-core system with very heavy
+// rename(2) load, the single longest latency caused by -EAGAIN retries was
+// ~800us (with the vast majority being closer to 400us). So, a 2ms limit
+// should give more than enough headroom for any real system in practice.
+const retryDeadline = 2 * time.Millisecond
+
+// retryEAGAIN is a top-level retry loop for pathrs to try to returning
+// spurious errors in most normal user cases when using openat2 (libpathrs
+// itself does up to 128 retries already, but this method takes a
+// wallclock-deadline approach to simply retry until a timer elapses).
+func retryEAGAIN[T any](fn func() (T, error)) (T, error) {
+	deadline := time.After(retryDeadline)
+	for {
+		v, err := fn()
+		if !errors.Is(err, unix.EAGAIN) {
+			return v, err
+		}
+		select {
+		case <-deadline:
+			return *new(T), fmt.Errorf("%v retry deadline exceeded: %w", retryDeadline, err)
+		default:
+			// retry
+		}
+	}
+}
+
+// retryEAGAIN2 is like retryEAGAIN except it returns two values.
+func retryEAGAIN2[T1, T2 any](fn func() (T1, T2, error)) (T1, T2, error) {
+	type ret struct {
+		v1 T1
+		v2 T2
+	}
+	v, err := retryEAGAIN(func() (ret, error) {
+		v1, v2, err := fn()
+		return ret{v1: v1, v2: v2}, err
+	})
+	return v.v1, v.v2, err
+}

internal/pathrs/root_pathrslite.go

@@ -31,12 +31,15 @@ import (
 // is effectively shorthand for [securejoin.OpenInRoot] followed by
 // [securejoin.Reopen].
 func OpenInRoot(root, subpath string, flags int) (*os.File, error) {
-	handle, err := pathrs.OpenInRoot(root, subpath)
+	handle, err := retryEAGAIN(func() (*os.File, error) {
+		return pathrs.OpenInRoot(root, subpath)
+	})
 	if err != nil {
 		return nil, err
 	}
 	defer handle.Close()
-	return pathrs.Reopen(handle, flags)
+
+	return Reopen(handle, flags)
 }
 
 // CreateInRoot creates a new file inside a root (as well as any missing parent