diff --git a/pathrs-lite/internal/errors.go b/pathrs-lite/internal/errors_linux.go similarity index 70% rename from pathrs-lite/internal/errors.go rename to pathrs-lite/internal/errors_linux.go index c26e440..d0b200f 100644 --- a/pathrs-lite/internal/errors.go +++ b/pathrs-lite/internal/errors_linux.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 +//go:build linux + // Copyright (C) 2024-2025 Aleksa Sarai // 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") diff --git a/pathrs-lite/internal/errors_linux_test.go b/pathrs-lite/internal/errors_linux_test.go new file mode 100644 index 0000000..9077746 --- /dev/null +++ b/pathrs-lite/internal/errors_linux_test.go @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MPL-2.0 + +//go:build linux + +// Copyright (C) 2024-2025 Aleksa Sarai +// Copyright (C) 2024-2025 SUSE LLC +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +package internal + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" +) + +func TestErrorXdev(t *testing.T) { + for _, test := range []struct { + name string + err error + }{ + {"ErrPossibleAttack", ErrPossibleAttack}, + {"ErrPossibleBreakout", ErrPossibleBreakout}, + } { + t.Run(test.name, func(t *testing.T) { + assert.ErrorIs(t, test.err, test.err, "errors.Is(err, err) should succeed") //nolint:useless-assert,testifylint // we need to check this + assert.ErrorIs(t, test.err, unix.EXDEV, "errors.Is(err, EXDEV) should succeed") //nolint:useless-assert,testifylint // we need to check this + }) + + t.Run(test.name+"-Wrapped", func(t *testing.T) { + err := fmt.Errorf("wrapped error: %w", test.err) + assert.ErrorIs(t, err, test.err, "errors.Is(err, err) should succeed") //nolint:useless-assert,testifylint // we need to check this + assert.ErrorIs(t, err, unix.EXDEV, "errors.Is(err, EXDEV) should succeed") //nolint:useless-assert,testifylint // we need to check this + }) + } +} diff --git a/pathrs-lite/internal/fd/openat2_linux.go b/pathrs-lite/internal/fd/openat2_linux.go index 2305308..3e937fe 100644 --- a/pathrs-lite/internal/fd/openat2_linux.go +++ b/pathrs-lite/internal/fd/openat2_linux.go @@ -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} } diff --git a/pathrs-lite/lookup_linux_test.go b/pathrs-lite/lookup_linux_test.go index bc6e5fe..8f75c6d 100644 --- a/pathrs-lite/lookup_linux_test.go +++ b/pathrs-lite/lookup_linux_test.go @@ -24,7 +24,6 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/sys/unix" - "github.com/cyphar/filepath-securejoin/pathrs-lite/internal" "github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd" "github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat" "github.com/cyphar/filepath-securejoin/pathrs-lite/internal/procfs" @@ -580,7 +579,7 @@ func TestPartialLookup_RacingRename(t *testing.T) { )}, } { test := test // copy iterator - test.skipErrs = append(test.skipErrs, internal.ErrPossibleAttack, internal.ErrPossibleBreakout) + test.skipErrs = append(test.skipErrs, unix.EAGAIN, unix.EXDEV) t.Run(name, func(t *testing.T) { root := testutils.CreateTree(t, tree...)