mirror of
https://github.com/opencontainers/runc.git
synced 2026-07-10 21:53:57 +08:00
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 <estesp@gmail.com> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This commit is contained in:
+32
-2
@@ -4,7 +4,36 @@ All notable changes to this project will be documented in this file.
|
||||
The format is based on [Keep a Changelog](http://keepachangelog.com/)
|
||||
and this project adheres to [Semantic Versioning](http://semver.org/).
|
||||
|
||||
## [Unreleased] ##
|
||||
## [Unreleased 0.5.z] ##
|
||||
|
||||
## [0.5.1] - 2025-10-31 ##
|
||||
|
||||
> Spooky scary skeletons send shivers down your spine!
|
||||
|
||||
### Changed ###
|
||||
- `openat2` can return `-EAGAIN` if it detects a possible attack in certain
|
||||
scenarios (namely if there was a rename or mount while walking a path with a
|
||||
`..` component). While this is necessary to avoid a denial-of-service in the
|
||||
kernel, it does require retry loops in userspace.
|
||||
|
||||
In previous versions, `pathrs-lite` would retry `openat2` 32 times before
|
||||
returning an error, but we've received user reports that this limit can be
|
||||
hit on systems with very heavy load. In some synthetic benchmarks (testing
|
||||
the worst-case of an attacker doing renames in a tight loop on every core of
|
||||
a 16-core machine) we managed to get a ~3% failure rate in runc. We have
|
||||
improved this situation in two ways:
|
||||
|
||||
* We have now increased this limit to 128, which should be good enough for
|
||||
most use-cases without becoming a denial-of-service vector (the number of
|
||||
syscalls called by the `O_PATH` resolver in a typical case is within the
|
||||
same ballpark). The same benchmarks show a failure rate of ~0.12% which
|
||||
(while not zero) is probably sufficient for most users.
|
||||
|
||||
* In addition, we now return a `unix.EAGAIN` error that is bubbled up and can
|
||||
be detected by callers. This means that callers with stricter requirements
|
||||
to avoid spurious errors can choose to do their own infinite `EAGAIN` retry
|
||||
loop (though we would strongly recommend users use time-based deadlines in
|
||||
such retry loops to avoid potentially unbounded denials-of-service).
|
||||
|
||||
## [0.5.0] - 2025-09-26 ##
|
||||
|
||||
@@ -354,7 +383,8 @@ This is our first release of `github.com/cyphar/filepath-securejoin`,
|
||||
containing a full implementation with a coverage of 93.5% (the only missing
|
||||
cases are the error cases, which are hard to mocktest at the moment).
|
||||
|
||||
[Unreleased]: https://github.com/cyphar/filepath-securejoin/compare/v0.5.0...HEAD
|
||||
[Unreleased 0.5.z]: https://github.com/cyphar/filepath-securejoin/compare/v0.5.1...release-0.5
|
||||
[0.5.1]: https://github.com/cyphar/filepath-securejoin/compare/v0.5.0...v0.5.1
|
||||
[0.5.0]: https://github.com/cyphar/filepath-securejoin/compare/v0.4.1...v0.5.0
|
||||
[0.4.1]: https://github.com/cyphar/filepath-securejoin/compare/v0.4.0...v0.4.1
|
||||
[0.4.0]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.6...v0.4.0
|
||||
|
||||
+1
-1
@@ -1 +1 @@
|
||||
0.5.0
|
||||
0.5.1
|
||||
|
||||
Generated
Vendored
+13
-2
@@ -1,5 +1,7 @@
|
||||
// SPDX-License-Identifier: MPL-2.0
|
||||
|
||||
//go:build linux
|
||||
|
||||
// Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
|
||||
// Copyright (C) 2024-2025 SUSE LLC
|
||||
//
|
||||
@@ -12,15 +14,24 @@ package internal
|
||||
|
||||
import (
|
||||
"errors"
|
||||
|
||||
"golang.org/x/sys/unix"
|
||||
)
|
||||
|
||||
type xdevErrorish struct {
|
||||
description string
|
||||
}
|
||||
|
||||
func (err xdevErrorish) Error() string { return err.description }
|
||||
func (err xdevErrorish) Is(target error) bool { return target == unix.EXDEV }
|
||||
|
||||
var (
|
||||
// ErrPossibleAttack indicates that some attack was detected.
|
||||
ErrPossibleAttack = errors.New("possible attack detected")
|
||||
ErrPossibleAttack error = xdevErrorish{"possible attack detected"}
|
||||
|
||||
// ErrPossibleBreakout indicates that during an operation we ended up in a
|
||||
// state that could be a breakout but we detected it.
|
||||
ErrPossibleBreakout = errors.New("possible breakout detected")
|
||||
ErrPossibleBreakout error = xdevErrorish{"possible breakout detected"}
|
||||
|
||||
// ErrInvalidDirectory indicates an unlinked directory.
|
||||
ErrInvalidDirectory = errors.New("wandered into deleted directory")
|
||||
Generated
Vendored
+6
-6
@@ -17,8 +17,6 @@ import (
|
||||
"runtime"
|
||||
|
||||
"golang.org/x/sys/unix"
|
||||
|
||||
"github.com/cyphar/filepath-securejoin/pathrs-lite/internal"
|
||||
)
|
||||
|
||||
func scopedLookupShouldRetry(how *unix.OpenHow, err error) bool {
|
||||
@@ -34,7 +32,10 @@ func scopedLookupShouldRetry(how *unix.OpenHow, err error) bool {
|
||||
(errors.Is(err, unix.EAGAIN) || errors.Is(err, unix.EXDEV))
|
||||
}
|
||||
|
||||
const scopedLookupMaxRetries = 32
|
||||
// This is a fairly arbitrary limit we have just to avoid an attacker being
|
||||
// able to make us spin in an infinite retry loop -- callers can choose to
|
||||
// retry on EAGAIN if they prefer.
|
||||
const scopedLookupMaxRetries = 128
|
||||
|
||||
// Openat2 is an [Fd]-based wrapper around unix.Openat2, but with some retry
|
||||
// logic in case of EAGAIN errors.
|
||||
@@ -43,10 +44,10 @@ func Openat2(dir Fd, path string, how *unix.OpenHow) (*os.File, error) {
|
||||
// Make sure we always set O_CLOEXEC.
|
||||
how.Flags |= unix.O_CLOEXEC
|
||||
var tries int
|
||||
for tries < scopedLookupMaxRetries {
|
||||
for {
|
||||
fd, err := unix.Openat2(dirFd, path, how)
|
||||
if err != nil {
|
||||
if scopedLookupShouldRetry(how, err) {
|
||||
if scopedLookupShouldRetry(how, err) && tries < scopedLookupMaxRetries {
|
||||
// We retry a couple of times to avoid the spurious errors, and
|
||||
// if we are being attacked then returning -EAGAIN is the best
|
||||
// we can do.
|
||||
@@ -58,5 +59,4 @@ func Openat2(dir Fd, path string, how *unix.OpenHow) (*os.File, error) {
|
||||
runtime.KeepAlive(dir)
|
||||
return os.NewFile(uintptr(fd), fullPath), nil
|
||||
}
|
||||
return nil, &os.PathError{Op: "openat2", Path: fullPath, Err: internal.ErrPossibleAttack}
|
||||
}
|
||||
|
||||
Vendored
+1
-1
@@ -27,7 +27,7 @@ github.com/coreos/go-systemd/v22/dbus
|
||||
# github.com/cpuguy83/go-md2man/v2 v2.0.7
|
||||
## explicit; go 1.12
|
||||
github.com/cpuguy83/go-md2man/v2/md2man
|
||||
# github.com/cyphar/filepath-securejoin v0.5.0
|
||||
# github.com/cyphar/filepath-securejoin v0.5.1
|
||||
## explicit; go 1.18
|
||||
github.com/cyphar/filepath-securejoin
|
||||
github.com/cyphar/filepath-securejoin/internal/consts
|
||||
|
||||
Reference in New Issue
Block a user