Skip to content

Commit 1bc551b

Browse files
committed
merge #73 into cyphar/filepath-securejoin:main
Aleksa Sarai (2): gopathrs: return raw errors from openat2 retry loop pathrs: support errors.Is(unix.EXDEV) checks for ErrPossible* LGTMs: cyphar
2 parents 987574e + 02d2e4b commit 1bc551b

File tree

4 files changed

+57
-9
lines changed

4 files changed

+57
-9
lines changed

pathrs-lite/internal/errors.go renamed to pathrs-lite/internal/errors_linux.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// SPDX-License-Identifier: MPL-2.0
22

3+
//go:build linux
4+
35
// Copyright (C) 2024-2025 Aleksa Sarai <[email protected]>
46
// Copyright (C) 2024-2025 SUSE LLC
57
//
@@ -12,15 +14,24 @@ package internal
1214

1315
import (
1416
"errors"
17+
18+
"golang.org/x/sys/unix"
1519
)
1620

21+
type xdevErrorish struct {
22+
description string
23+
}
24+
25+
func (err xdevErrorish) Error() string { return err.description }
26+
func (err xdevErrorish) Is(target error) bool { return target == unix.EXDEV }
27+
1728
var (
1829
// ErrPossibleAttack indicates that some attack was detected.
19-
ErrPossibleAttack = errors.New("possible attack detected")
30+
ErrPossibleAttack error = xdevErrorish{"possible attack detected"}
2031

2132
// ErrPossibleBreakout indicates that during an operation we ended up in a
2233
// state that could be a breakout but we detected it.
23-
ErrPossibleBreakout = errors.New("possible breakout detected")
34+
ErrPossibleBreakout error = xdevErrorish{"possible breakout detected"}
2435

2536
// ErrInvalidDirectory indicates an unlinked directory.
2637
ErrInvalidDirectory = errors.New("wandered into deleted directory")
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
3+
//go:build linux
4+
5+
// Copyright (C) 2024-2025 Aleksa Sarai <[email protected]>
6+
// Copyright (C) 2024-2025 SUSE LLC
7+
//
8+
// This Source Code Form is subject to the terms of the Mozilla Public
9+
// License, v. 2.0. If a copy of the MPL was not distributed with this
10+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
11+
12+
package internal
13+
14+
import (
15+
"fmt"
16+
"testing"
17+
18+
"github.com/stretchr/testify/assert"
19+
"golang.org/x/sys/unix"
20+
)
21+
22+
func TestErrorXdev(t *testing.T) {
23+
for _, test := range []struct {
24+
name string
25+
err error
26+
}{
27+
{"ErrPossibleAttack", ErrPossibleAttack},
28+
{"ErrPossibleBreakout", ErrPossibleBreakout},
29+
} {
30+
t.Run(test.name, func(t *testing.T) {
31+
assert.ErrorIs(t, test.err, test.err, "errors.Is(err, err) should succeed") //nolint:useless-assert,testifylint // we need to check this
32+
assert.ErrorIs(t, test.err, unix.EXDEV, "errors.Is(err, EXDEV) should succeed") //nolint:useless-assert,testifylint // we need to check this
33+
})
34+
35+
t.Run(test.name+"-Wrapped", func(t *testing.T) {
36+
err := fmt.Errorf("wrapped error: %w", test.err)
37+
assert.ErrorIs(t, err, test.err, "errors.Is(err, err) should succeed") //nolint:useless-assert,testifylint // we need to check this
38+
assert.ErrorIs(t, err, unix.EXDEV, "errors.Is(err, EXDEV) should succeed") //nolint:useless-assert,testifylint // we need to check this
39+
})
40+
}
41+
}

pathrs-lite/internal/fd/openat2_linux.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ import (
1717
"runtime"
1818

1919
"golang.org/x/sys/unix"
20-
21-
"github.com/cyphar/filepath-securejoin/pathrs-lite/internal"
2220
)
2321

2422
func scopedLookupShouldRetry(how *unix.OpenHow, err error) bool {
@@ -43,10 +41,10 @@ func Openat2(dir Fd, path string, how *unix.OpenHow) (*os.File, error) {
4341
// Make sure we always set O_CLOEXEC.
4442
how.Flags |= unix.O_CLOEXEC
4543
var tries int
46-
for tries < scopedLookupMaxRetries {
44+
for {
4745
fd, err := unix.Openat2(dirFd, path, how)
4846
if err != nil {
49-
if scopedLookupShouldRetry(how, err) {
47+
if scopedLookupShouldRetry(how, err) && tries < scopedLookupMaxRetries {
5048
// We retry a couple of times to avoid the spurious errors, and
5149
// if we are being attacked then returning -EAGAIN is the best
5250
// we can do.
@@ -58,5 +56,4 @@ func Openat2(dir Fd, path string, how *unix.OpenHow) (*os.File, error) {
5856
runtime.KeepAlive(dir)
5957
return os.NewFile(uintptr(fd), fullPath), nil
6058
}
61-
return nil, &os.PathError{Op: "openat2", Path: fullPath, Err: internal.ErrPossibleAttack}
6259
}

pathrs-lite/internal/gopathrs/lookup_linux_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"github.com/stretchr/testify/require"
2525
"golang.org/x/sys/unix"
2626

27-
"github.com/cyphar/filepath-securejoin/pathrs-lite/internal"
2827
"github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd"
2928
"github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat"
3029
"github.com/cyphar/filepath-securejoin/pathrs-lite/internal/procfs"
@@ -580,7 +579,7 @@ func TestPartialLookup_RacingRename(t *testing.T) {
580579
)},
581580
} {
582581
test := test // copy iterator
583-
test.skipErrs = append(test.skipErrs, internal.ErrPossibleAttack, internal.ErrPossibleBreakout)
582+
test.skipErrs = append(test.skipErrs, unix.EAGAIN, unix.EXDEV)
584583
t.Run(name, func(t *testing.T) {
585584
root := testutils.CreateTree(t, tree...)
586585

0 commit comments

Comments
 (0)