cyphar · Jul 11, 2024 · Jul 12, 2024 · Jul 12, 2024 · Jul 12, 2024 · Jul 15, 2024
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
@@ -0,0 +1,16 @@
+# Please see the documentation for all configuration options:
+# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates
+
+version: 2
+updates:
+  # Dependencies in go.mod.
+  - package-ecosystem: "gomod"
+    directory: "/"
+    schedule:
+      interval: "daily"
+
+  # Dependencies in .github/workflows/*.yml.
+  - package-ecosystem: "github-actions"
+    directory: "/"
+    schedule:
+      interval: "daily"
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -22,7 +22,7 @@ jobs:
     runs-on: windows-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-go@v4
+      - uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.go-version }}
       - name: mkdir gocoverdir
@@ -49,7 +49,7 @@ jobs:
           name: coverage-${{ runner.os }}-${{ github.job }}-${{ strategy.job-index }}
           path: ${{ env.GOCOVERDIR }}
 
-  test-unix-short:
+  test-unix:
     strategy:
       fail-fast: false
       matrix:
@@ -67,42 +67,31 @@ jobs:
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-go@v4
+      - uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.go-version }}
       - name: mkdir gocoverdir
         run: |
           GOCOVERDIR="$(mktemp --tmpdir -d gocoverdir.XXXXXXXX)"
           echo "GOCOVERDIR=$GOCOVERDIR" >>"$GITHUB_ENV"
-      - name: unit tests
-        run: |
-          go test -short -v -cover -test.gocoverdir="$GOCOVERDIR" ./...
-          sudo go test -short -v -cover -test.gocoverdir="$GOCOVERDIR" ./...
+      - name: go test
+        run: go test -v -cover -test.gocoverdir="$GOCOVERDIR" ./...
+      - name: sudo go test
+        run: sudo go test -v -cover -test.gocoverdir="$GOCOVERDIR" ./...
       - name: upload coverage
         uses: actions/upload-artifact@v4
         with:
           name: coverage-${{ runner.os }}-${{ github.job }}-${{ strategy.job-index }}
           path: ${{ env.GOCOVERDIR }}
 
-  test-linux-stress:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-go@v4
-        with:
-          go-version: "^1"
-      - name: stress test race protections
-        run: |
-          sudo go test -timeout 1h -v -cover -run 'Test.*Racing' ./...
-
   coverage:
     runs-on: ubuntu-latest
     needs:
       - test-windows
-      - test-unix-short
+      - test-unix
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-go@v4
+      - uses: actions/setup-go@v5
         with:
           go-version: "^1"
       - name: download all coverage

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,138 @@
+# Changelog #
+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] ##
+
+## [0.3.1] - 2024-07-23 ##
+
+### Changed ###
+- By allowing `Open(at)InRoot` to opt-out of the extra work done by `MkdirAll`
+  to do the necessary "partial lookups", `Open(at)InRoot` now does less work
+  for both implementations (resulting in a many-fold decrease in the number of
+  operations for `openat2`, and a modest improvement for non-`openat2`) and is
+  far more guaranteed to match the correct `openat2(RESOLVE_IN_ROOT)`
+  behaviour.
+- We now use `readlinkat(fd, "")` where possible. For `Open(at)InRoot` this
+  effectively just means that we no longer risk getting spurious errors during
+  rename races. However, for our hardened procfs handler, this in theory should
+  prevent mount attacks from tricking us when doing magic-link readlinks (even
+  when using the unsafe host `/proc` handle). Unfortunately `Reopen` is still
+  potentially vulnerable to those kinds of somewhat-esoteric attacks.
+
+  Technically this [will only work on post-2.6.39 kernels][linux-readlinkat-emptypath]
+  but it seems incredibly unlikely anyone is using `filepath-securejoin` on a
+  pre-2011 kernel.
+
+### Fixed ###
+- Several improvements were made to the errors returned by `Open(at)InRoot` and
+  `MkdirAll` when dealing with invalid paths under the emulated (ie.
+  non-`openat2`) implementation. Previously, some paths would return the wrong
+  error (`ENOENT` when the last component was a non-directory), and other paths
+  would be returned as though they were acceptable (trailing-slash components
+  after a non-directory would be ignored by `Open(at)InRoot`).
+
+  These changes were done to match `openat2`'s behaviour and purely is a
+  consistency fix (most users are going to be using `openat2` anyway).
+
+[linux-readlinkat-emptypath]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65cfc6722361570bfe255698d9cd4dccaf47570d
+
+## [0.3.0] - 2024-07-11 ##
+
+### Added ###
+- A new set of `*os.File`-based APIs have been added. These are adapted from
+  [libpathrs][] and we strongly suggest using them if possible (as they provide
+  far more protection against attacks than `SecureJoin`):
+
+   - `Open(at)InRoot` resolves a path inside a rootfs and returns an `*os.File`
+     handle to the path. Note that the handle returned is an `O_PATH` handle,
+     which cannot be used for reading or writing (as well as some other
+     operations -- [see open(2) for more details][open.2])
+
+   - `Reopen` takes an `O_PATH` file handle and safely re-opens it to upgrade
+     it to a regular handle. This can also be used with non-`O_PATH` handles,
+     but `O_PATH` is the most obvious application.
+
+   - `MkdirAll` is an implementation of `os.MkdirAll` that is safe to use to
+     create a directory tree within a rootfs.
+
+  As these are new APIs, they may change in the future. However, they should be
+  safe to start migrating to as we have extensive tests ensuring they behave
+  correctly and are safe against various races and other attacks.
+
+[libpathrs]: https://github.com/openSUSE/libpathrs
+[open.2]: https://www.man7.org/linux/man-pages/man2/open.2.html
+
+## [0.2.5] - 2024-05-03 ##
+
+### Changed ###
+- Some minor changes were made to how lexical components (like `..` and `.`)
+  are handled during path generation in `SecureJoin`. There is no behaviour
+  change as a result of this fix (the resulting paths are the same).
+
+### Fixed ###
+- The error returned when we hit a symlink loop now references the correct
+  path. (#10)
+
+## [0.2.4] - 2023-09-06 ##
+
+### Security ###
+- This release fixes a potential security issue in filepath-securejoin when
+  used on Windows ([GHSA-6xv5-86q9-7xr8][], which could be used to generate
+  paths outside of the provided rootfs in certain cases), as well as improving
+  the overall behaviour of filepath-securejoin when dealing with Windows paths
+  that contain volume names. Thanks to Paulo Gomes for discovering and fixing
+  these issues.
+
+### Fixed ###
+- Switch to GitHub Actions for CI so we can test on Windows as well as Linux
+  and MacOS.
+
+[GHSA-6xv5-86q9-7xr8]: https://github.com/advisories/GHSA-6xv5-86q9-7xr8
+
+## [0.2.3] - 2021-06-04 ##
+
+### Changed ###
+- Switch to Go 1.13-style `%w` error wrapping, letting us drop the dependency
+  on `github.com/pkg/errors`.
+
+## [0.2.2] - 2018-09-05 ##
+
+### Changed ###
+- Use `syscall.ELOOP` as the base error for symlink loops, rather than our own
+  (internal) error. This allows callers to more easily use `errors.Is` to check
+  for this case.
+
+## [0.2.1] - 2018-09-05 ##
+
+### Fixed ###
+- Use our own `IsNotExist` implementation, which lets us handle `ENOTDIR`
+  properly within `SecureJoin`.
+
+## [0.2.0] - 2017-07-19 ##
+
+We now have 100% test coverage!
+
+### Added ###
+- Add a `SecureJoinVFS` API that can be used for mocking (as we do in our new
+  tests) or for implementing custom handling of lookup operations (such as for
+  rootless containers, where work is necessary to access directories with weird
+  modes because we don't have `CAP_DAC_READ_SEARCH` or `CAP_DAC_OVERRIDE`).
+
+## 0.1.0 - 2017-07-19
+
+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.3.1...HEAD
+[0.3.1]: https://github.com/cyphar/filepath-securejoin/compare/v0.3.0...v0.3.1
+[0.3.0]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.5...v0.3.0
+[0.2.5]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.4...v0.2.5
+[0.2.4]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.3...v0.2.4
+[0.2.3]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.2...v0.2.3
+[0.2.2]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.1...v0.2.2
+[0.2.1]: https://github.com/cyphar/filepath-securejoin/compare/v0.2.0...v0.2.1
+[0.2.0]: https://github.com/cyphar/filepath-securejoin/compare/v0.1.0...v0.2.0
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-0.3.0
+0.3.1
diff --git a/lookup_linux.go b/lookup_linux.go
@@ -40,16 +40,18 @@ func (se symlinkStackEntry) Close() {
 
 type symlinkStack []*symlinkStackEntry
 
-func (s symlinkStack) IsEmpty() bool {
-	return len(s) == 0
+func (s *symlinkStack) IsEmpty() bool {
+	return s == nil || len(*s) == 0
 }
 
 func (s *symlinkStack) Close() {
-	for _, link := range *s {
-		link.Close()
+	if s != nil {
+		for _, link := range *s {
+			link.Close()
+		}
+		// TODO: Switch to clear once we switch to Go 1.21.
+		*s = nil
 	}
-	// TODO: Switch to clear once we switch to Go 1.21.
-	*s = nil
 }
 
 var (
@@ -58,11 +60,16 @@ var (
 )
 
 func (s *symlinkStack) popPart(part string) error {
-	if s.IsEmpty() {
+	if s == nil || s.IsEmpty() {
 		// If there is nothing in the symlink stack, then the part was from the
 		// real path provided by the user, and this is a no-op.
 		return errEmptyStack
 	}
+	if part == "." {
+		// "." components are no-ops -- we drop them when doing SwapLink.
+		return nil
+	}
+
 	tailEntry := (*s)[len(*s)-1]
 
 	// Double-check that we are popping the component we expect.
@@ -102,17 +109,13 @@ func (s *symlinkStack) PopPart(part string) error {
 }
 
 func (s *symlinkStack) push(dir *os.File, remainingPath, linkTarget string) error {
+	if s == nil {
+		return nil
+	}
 	// Split the link target and clean up any "" parts.
 	linkTargetParts := slices.DeleteFunc(
 		strings.Split(linkTarget, "/"),
-		func(part string) bool { return part == "" })
-
-	// Don't add a no-op link to the stack. You can't create a no-op link
-	// symlink, but if the symlink is /, partialLookupInRoot has already jumped to the
-	// root and so there's nothing more to do.
-	if len(linkTargetParts) == 0 {
-		return nil
-	}
+		func(part string) bool { return part == "" || part == "." })
 
 	// Copy the directory so the caller doesn't close our copy.
 	dirCopy, err := dupFile(dir)
@@ -145,7 +148,7 @@ func (s *symlinkStack) SwapLink(linkPart string, dir *os.File, remainingPath, li
 }
 
 func (s *symlinkStack) PopTopSymlink() (*os.File, string, bool) {
-	if s.IsEmpty() {
+	if s == nil || s.IsEmpty() {
 		return nil, "", false
 	}
 	tailEntry := (*s)[0]
@@ -157,7 +160,22 @@ func (s *symlinkStack) PopTopSymlink() (*os.File, string, bool) {
 // within the provided root (a-la RESOLVE_IN_ROOT) and opens the final existing
 // component of the requested path, returning a file handle to the final
 // existing component and a string containing the remaining path components.
-func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string, Err error) {
+func partialLookupInRoot(root *os.File, unsafePath string) (*os.File, string, error) {
+	return lookupInRoot(root, unsafePath, true)
+}
+
+func completeLookupInRoot(root *os.File, unsafePath string) (*os.File, error) {
+	handle, remainingPath, err := lookupInRoot(root, unsafePath, false)
+	if remainingPath != "" && err == nil {
+		// should never happen
+		err = fmt.Errorf("[bug] non-empty remaining path when doing a non-partial lookup: %q", remainingPath)
+	}
+	// lookupInRoot(partial=false) will always close the handle if an error is
+	// returned, so no need to double-check here.
+	return handle, err
+}
+
+func lookupInRoot(root *os.File, unsafePath string, partial bool) (Handle *os.File, _ string, _ error) {
 	unsafePath = filepath.ToSlash(unsafePath) // noop
 
 	// This is very similar to SecureJoin, except that we operate on the
@@ -166,7 +184,7 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 
 	// Try to use openat2 if possible.
 	if hasOpenat2() {
-		return partialLookupOpenat2(root, unsafePath)
+		return lookupOpenat2(root, unsafePath, partial)
 	}
 
 	// Get the "actual" root path from /proc/self/fd. This is necessary if the
@@ -183,7 +201,8 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 		return nil, "", fmt.Errorf("clone root fd: %w", err)
 	}
 	defer func() {
-		if Err != nil {
+		// If a handle is not returned, close the internal handle.
+		if Handle == nil {
 			_ = currentDir.Close()
 		}
 	}()
@@ -200,8 +219,11 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 	// Note that the stack is ONLY used for book-keeping. All of the actual
 	// path walking logic is still based on currentPath/remainingPath and
 	// currentDir (as in SecureJoin).
-	var symlinkStack symlinkStack
-	defer symlinkStack.Close()
+	var symStack *symlinkStack
+	if partial {
+		symStack = new(symlinkStack)
+		defer symStack.Close()
+	}
 
 	var (
 		linksWalked   int
@@ -220,9 +242,11 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 		} else {
 			part, remainingPath = remainingPath[:i], remainingPath[i+1:]
 		}
-		// Skip any "//" components.
+		// If we hit an empty component, we need to treat it as though it is
+		// "." so that trailing "/" and "//" components on a non-directory
+		// correctly return the right error code.
 		if part == "" {
-			continue
+			part = "."
 		}
 
 		// Apply the component lexically to the path we are building.
@@ -233,7 +257,7 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 		// If we logically hit the root, just clone the root rather than
 		// opening the part and doing all of the other checks.
 		if nextPath == "/" {
-			if err := symlinkStack.PopPart(part); err != nil {
+			if err := symStack.PopPart(part); err != nil {
 				return nil, "", fmt.Errorf("walking into root with part %q failed: %w", part, err)
 			}
 			// Jump to root.
@@ -258,63 +282,24 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 			}
 
 			switch st.Mode() & os.ModeType {
-			case os.ModeDir:
-				// If we are dealing with a directory, simply walk into it.
-				_ = currentDir.Close()
-				currentDir = nextDir
-				currentPath = nextPath
-
-				// The part was real, so drop it from the symlink stack.
-				if err := symlinkStack.PopPart(part); err != nil {
-					return nil, "", fmt.Errorf("walking into directory %q failed: %w", part, err)
-				}
-
-				// If we are operating on a .., make sure we haven't escaped.
-				// We only have to check for ".." here because walking down
-				// into a regular component component cannot cause you to
-				// escape. This mirrors the logic in RESOLVE_IN_ROOT, except we
-				// have to check every ".." rather than only checking after a
-				// rename or mount on the system.
-				if part == ".." {
-					// Make sure the root hasn't moved.
-					if err := checkProcSelfFdPath(logicalRootPath, root); err != nil {
-						return nil, "", fmt.Errorf("root path moved during lookup: %w", err)
-					}
-					// Make sure the path is what we expect.
-					fullPath := logicalRootPath + nextPath
-					if err := checkProcSelfFdPath(fullPath, currentDir); err != nil {
-						return nil, "", fmt.Errorf("walking into %q had unexpected result: %w", part, err)
-					}
-				}
-
 			case os.ModeSymlink:
+				// readlinkat implies AT_EMPTY_PATH since Linux 2.6.39. See
+				// Linux commit 65cfc6722361 ("readlinkat(), fchownat() and
+				// fstatat() with empty relative pathnames").
+				linkDest, err := readlinkatFile(nextDir, "")
 				// We don't need the handle anymore.
 				_ = nextDir.Close()
-
-				// Unfortunately, we cannot readlink through our handle and so
-				// we need to do a separate readlinkat (which could race to
-				// give us an error if the attacker swapped the symlink with a
-				// non-symlink).
-				linkDest, err := readlinkatFile(currentDir, part)
 				if err != nil {
-					if errors.Is(err, unix.EINVAL) {
-						// The part was not a symlink, so assume that it's a
-						// regular file. It is possible for it to be a
-						// directory (if an attacker is swapping a directory
-						// and non-directory at this subpath) but erroring out
-						// here is better anyway.
-						err = fmt.Errorf("%w: path component %q is invalid: %w", errPossibleAttack, part, unix.ENOTDIR)
-					}
 					return nil, "", err
 				}
 
 				linksWalked++
 				if linksWalked > maxSymlinkLimit {
-					return nil, "", &os.PathError{Op: "partialLookupInRoot", Path: logicalRootPath + "/" + unsafePath, Err: unix.ELOOP}
+					return nil, "", &os.PathError{Op: "securejoin.lookupInRoot", Path: logicalRootPath + "/" + unsafePath, Err: unix.ELOOP}
 				}
 
 				// Swap out the symlink's component for the link entry itself.
-				if err := symlinkStack.SwapLink(part, currentDir, oldRemainingPath, linkDest); err != nil {
+				if err := symStack.SwapLink(part, currentDir, oldRemainingPath, linkDest); err != nil {
 					return nil, "", fmt.Errorf("walking into symlink %q failed: push symlink: %w", part, err)
 				}
 
@@ -331,50 +316,74 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 					currentDir = rootClone
 					currentPath = "/"
 				}
+
 			default:
-				// For any other file type, we can't walk further and so we've
-				// hit the end of the lookup. The handling is very similar to
-				// ENOENT from openat(2), except that we return a handle to the
-				// component we just walked into (and we drop the component
-				// from the symlink stack).
+				// If we are dealing with a directory, simply walk into it.
 				_ = currentDir.Close()
+				currentDir = nextDir
+				currentPath = nextPath
 
-				// The part existed, so drop it from the symlink stack.
-				if err := symlinkStack.PopPart(part); err != nil {
-					return nil, "", fmt.Errorf("walking into non-directory %q failed: %w", part, err)
+				// The part was real, so drop it from the symlink stack.
+				if err := symStack.PopPart(part); err != nil {
+					return nil, "", fmt.Errorf("walking into directory %q failed: %w", part, err)
 				}
 
-				// If there are any remaining components in the symlink stack,
-				// we are still within a symlink resolution and thus we hit a
-				// dangling symlink. So pretend that the first symlink in the
-				// stack we hit was an ENOENT (to match openat2).
-				if oldDir, remainingPath, ok := symlinkStack.PopTopSymlink(); ok {
-					_ = nextDir.Close()
-					return oldDir, remainingPath, nil
+				// If we are operating on a .., make sure we haven't escaped.
+				// We only have to check for ".." here because walking down
+				// into a regular component component cannot cause you to
+				// escape. This mirrors the logic in RESOLVE_IN_ROOT, except we
+				// have to check every ".." rather than only checking after a
+				// rename or mount on the system.
+				if part == ".." {
+					// Make sure the root hasn't moved.
+					if err := checkProcSelfFdPath(logicalRootPath, root); err != nil {
+						return nil, "", fmt.Errorf("root path moved during lookup: %w", err)
+					}
+					// Make sure the path is what we expect.
+					fullPath := logicalRootPath + nextPath
+					if err := checkProcSelfFdPath(fullPath, currentDir); err != nil {
+						return nil, "", fmt.Errorf("walking into %q had unexpected result: %w", part, err)
+					}
 				}
-
-				// The current component exists, so return it.
-				return nextDir, remainingPath, nil
 			}
 
-		case errors.Is(err, os.ErrNotExist):
+		default:
+			if !partial {
+				return nil, "", err
+			}
 			// If there are any remaining components in the symlink stack, we
 			// are still within a symlink resolution and thus we hit a dangling
 			// symlink. So pretend that the first symlink in the stack we hit
 			// was an ENOENT (to match openat2).
-			if oldDir, remainingPath, ok := symlinkStack.PopTopSymlink(); ok {
+			if oldDir, remainingPath, ok := symStack.PopTopSymlink(); ok {
 				_ = currentDir.Close()
-				return oldDir, remainingPath, nil
+				return oldDir, remainingPath, err
 			}
 			// We have hit a final component that doesn't exist, so we have our
 			// partial open result. Note that we have to use the OLD remaining
 			// path, since the lookup failed.
-			return currentDir, oldRemainingPath, nil
+			return currentDir, oldRemainingPath, err
+		}
+	}
 
-		default:
-			return nil, "", err
+	// If the unsafePath had a trailing slash, we need to make sure we try to
+	// do a relative "." open so that we will correctly return an error when
+	// the final component is a non-directory (to match openat2). In the
+	// context of openat2, a trailing slash and a trailing "/." are completely
+	// equivalent.
+	if strings.HasSuffix(unsafePath, "/") {
+		nextDir, err := openatFile(currentDir, ".", unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
+		if err != nil {
+			if !partial {
+				_ = currentDir.Close()
+				currentDir = nil
+			}
+			return currentDir, "", err
 		}
+		_ = currentDir.Close()
+		currentDir = nextDir
 	}
+
 	// All of the components existed!
 	return currentDir, "", nil
 }
diff --git a/lookup_linux_test.go b/lookup_linux_test.go
diff --git a/mkdir_linux.go b/mkdir_linux.go
@@ -49,14 +49,14 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
 
 	// Try to open as much of the path as possible.
 	currentDir, remainingPath, err := partialLookupInRoot(root, unsafePath)
-	if err != nil {
-		return nil, fmt.Errorf("find existing subpath of %q: %w", unsafePath, err)
-	}
 	defer func() {
 		if Err != nil {
 			_ = currentDir.Close()
 		}
 	}()
+	if err != nil && !errors.Is(err, unix.ENOENT) {
+		return nil, fmt.Errorf("find existing subpath of %q: %w", unsafePath, err)
+	}
 
 	// If there is an attacker deleting directories as we walk into them,
 	// detect this proactively. Note this is guaranteed to detect if the
@@ -82,6 +82,7 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
 	} else if err != nil {
 		return nil, fmt.Errorf("re-opening handle to %q: %w", currentDir.Name(), err)
 	} else {
+		_ = currentDir.Close()
 		currentDir = reopenDir
 	}
 

diff --git a/mkdir_linux_test.go b/mkdir_linux_test.go
@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -358,16 +359,15 @@ func TestMkdirAllHandle_RacingRename(t *testing.T) {
 				}(rootCh)
 
 				// Do several runs to try to catch bugs.
-				var testRuns = 10000
-				if testing.Short() {
-					testRuns = 300
-				}
+				const testRuns = 2000
 				m := newRacingMkdirMeta()
 				for i := 0; i < testRuns; i++ {
 					root := createTree(t, treeSpec...)
-					rootCh <- root
 
+					rootCh <- root
+					runtime.Gosched() // give the thread some time to do a rename
 					m.checkMkdirAllHandle_Racing(t, root, test.unsafePath, 0o711, test.allowedErrs)
+					rootCh <- ""
 
 					// Clean up the root after each run so we don't exhaust all
 					// space in the tmpfs.
@@ -430,16 +430,14 @@ func TestMkdirAllHandle_RacingDelete(t *testing.T) {
 				}(rootCh)
 
 				// Do several runs to try to catch bugs.
-				var testRuns = 10000
-				if testing.Short() {
-					testRuns = 300
-				}
+				const testRuns = 2000
 				m := newRacingMkdirMeta()
 				for i := 0; i < testRuns; i++ {
 					root := createTree(t, treeSpec...)
-					rootCh <- root
 
+					rootCh <- root
 					m.checkMkdirAllHandle_Racing(t, root, test.unsafePath, 0o711, test.allowedErrs)
+					rootCh <- ""
 
 					// Clean up the root after each run so we don't exhaust all
 					// space in the tmpfs.

diff --git a/open_linux.go b/open_linux.go
@@ -9,20 +9,17 @@ package securejoin
 import (
 	"fmt"
 	"os"
+	"strconv"
 
 	"golang.org/x/sys/unix"
 )
 
 // OpenatInRoot is equivalent to OpenInRoot, except that the root is provided
 // using an *os.File handle, to ensure that the correct root directory is used.
 func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) {
-	handle, remainingPath, err := partialLookupInRoot(root, unsafePath)
+	handle, err := completeLookupInRoot(root, unsafePath)
 	if err != nil {
-		return nil, err
-	}
-	if remainingPath != "" {
-		_ = handle.Close()
-		return nil, &os.PathError{Op: "securejoin.OpenInRoot", Path: unsafePath, Err: unix.ENOENT}
+		return nil, &os.PathError{Op: "securejoin.OpenInRoot", Path: unsafePath, Err: err}
 	}
 	return handle, nil
 }
@@ -69,15 +66,36 @@ func Reopen(handle *os.File, flags int) (*os.File, error) {
 		return nil, err
 	}
 
+	// We can't operate on /proc/thread-self/fd/$n directly when doing a
+	// re-open, so we need to open /proc/thread-self/fd and then open a single
+	// final component.
+	procFdDir, closer, err := procThreadSelf(procRoot, "fd/")
+	if err != nil {
+		return nil, fmt.Errorf("get safe /proc/thread-self/fd handle: %w", err)
+	}
+	defer procFdDir.Close()
+	defer closer()
+
+	// Try to detect if there is a mount on top of the magic-link we are about
+	// to open. If we are using unsafeHostProcRoot(), this could change after
+	// we check it (and there's nothing we can do about that) but for
+	// privateProcRoot() this should be guaranteed to be safe (at least since
+	// Linux 5.12[1], when anonymous mount namespaces were completely isolated
+	// from external mounts including mount propagation events).
+	//
+	// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
+	// onto targets that reside on shared mounts").
+	fdStr := strconv.Itoa(int(handle.Fd()))
+	if err := checkSymlinkOvermount(procRoot, procFdDir, fdStr); err != nil {
+		return nil, fmt.Errorf("check safety of /proc/thread-self/fd/%s magiclink: %w", fdStr, err)
+	}
+
 	flags |= unix.O_CLOEXEC
-	fdPath := fmt.Sprintf("fd/%d", handle.Fd())
-	return doProcSelfMagiclink(procRoot, fdPath, func(procDirHandle *os.File, base string) (*os.File, error) {
-		// Rather than just wrapping openatFile, open-code it so we can copy
-		// handle.Name().
-		reopenFd, err := unix.Openat(int(procDirHandle.Fd()), base, flags, 0)
-		if err != nil {
-			return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err)
-		}
-		return os.NewFile(uintptr(reopenFd), handle.Name()), nil
-	})
+	// Rather than just wrapping openatFile, open-code it so we can copy
+	// handle.Name().
+	reopenFd, err := unix.Openat(int(procFdDir.Fd()), fdStr, flags, 0)
+	if err != nil {
+		return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err)
+	}
+	return os.NewFile(uintptr(reopenFd), handle.Name()), nil
 }
diff --git a/open_linux_test.go b/open_linux_test.go
@@ -209,60 +209,93 @@ func testOpenInRoot(t *testing.T, openInRootFn openInRootFunc) {
 		expected   openResult
 	}{
 		// Complete lookups.
-		"complete-dir1": {"a", openResult{handlePath: "/a", fileType: unix.S_IFDIR}},
-		"complete-dir2": {"b/c/d/e/f", openResult{handlePath: "/b/c/d/e/f", fileType: unix.S_IFDIR}},
-		"complete-fifo": {"b/fifo", openResult{handlePath: "/b/fifo", fileType: unix.S_IFIFO}},
-		"complete-sock": {"b/sock", openResult{handlePath: "/b/sock", fileType: unix.S_IFSOCK}},
+		"complete-dir1":      {"a", openResult{handlePath: "/a", fileType: unix.S_IFDIR}},
+		"complete-dir2":      {"b/c/d/e/f", openResult{handlePath: "/b/c/d/e/f", fileType: unix.S_IFDIR}},
+		"complete-dir3":      {"b///././c////.//d/./././///e////.//./f//././././", openResult{handlePath: "/b/c/d/e/f", fileType: unix.S_IFDIR}},
+		"complete-file":      {"b/c/file", openResult{handlePath: "/b/c/file", fileType: unix.S_IFREG}},
+		"complete-file-link": {"b-file", openResult{handlePath: "/b/c/file", fileType: unix.S_IFREG}},
+		"complete-fifo":      {"b/fifo", openResult{handlePath: "/b/fifo", fileType: unix.S_IFIFO}},
+		"complete-sock":      {"b/sock", openResult{handlePath: "/b/sock", fileType: unix.S_IFSOCK}},
 		// Partial lookups.
 		"partial-dir-basic":  {"a/b/c/d/e/f/g/h", openResult{err: unix.ENOENT}},
 		"partial-dir-dotdot": {"a/foo/../bar/baz", openResult{err: unix.ENOENT}},
 		// Complete lookups of non-lexical symlinks.
-		"nonlexical-basic-complete":                 {"target", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-basic-complete1":                {"target", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-basic-complete2":                {"target/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-basic-complete3":                {"target//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-basic-partial":                  {"target/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-basic-partial-dotdot":           {"target/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level1-abs-complete":            {"link1/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level1-abs-complete1":           {"link1/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level1-abs-complete2":           {"link1/target_abs/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level1-abs-complete3":           {"link1/target_abs//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level1-abs-partial":             {"link1/target_abs/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level1-abs-partial-dotdot":      {"link1/target_abs/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level1-rel-complete":            {"link1/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level1-rel-complete1":           {"link1/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level1-rel-complete2":           {"link1/target_rel/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level1-rel-complete3":           {"link1/target_rel//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level1-rel-partial":             {"link1/target_rel/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level1-rel-partial-dotdot":      {"link1/target_rel/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level2-abs-abs-complete":        {"link2/link1_abs/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-abs-complete1":       {"link2/link1_abs/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-abs-complete2":       {"link2/link1_abs/target_abs/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-abs-complete3":       {"link2/link1_abs/target_abs//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level2-abs-abs-partial":         {"link2/link1_abs/target_abs/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level2-abs-abs-partial-dotdot":  {"link2/link1_abs/target_abs/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level2-abs-rel-complete":        {"link2/link1_abs/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-rel-complete1":       {"link2/link1_abs/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-rel-complete2":       {"link2/link1_abs/target_rel/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-rel-complete3":       {"link2/link1_abs/target_rel//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level2-abs-rel-partial":         {"link2/link1_abs/target_rel/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level2-abs-rel-partial-dotdot":  {"link2/link1_abs/target_rel/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level2-abs-open-complete":       {"link2/link1_abs/../target", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-open-complete1":      {"link2/link1_abs/../target", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-open-complete2":      {"link2/link1_abs/../target/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-abs-open-complete3":      {"link2/link1_abs/../target//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level2-abs-open-partial":        {"link2/link1_abs/../target/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level2-abs-open-partial-dotdot": {"link2/link1_abs/../target/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level2-rel-abs-complete":        {"link2/link1_rel/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-abs-complete1":       {"link2/link1_rel/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-abs-complete2":       {"link2/link1_rel/target_abs/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-abs-complete3":       {"link2/link1_rel/target_abs//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level2-rel-abs-partial":         {"link2/link1_rel/target_abs/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level2-rel-abs-partial-dotdot":  {"link2/link1_rel/target_abs/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level2-rel-rel-complete":        {"link2/link1_rel/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-rel-complete1":       {"link2/link1_rel/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-rel-complete2":       {"link2/link1_rel/target_rel/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-rel-complete3":       {"link2/link1_rel/target_rel//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level2-rel-rel-partial":         {"link2/link1_rel/target_rel/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level2-rel-rel-partial-dotdot":  {"link2/link1_rel/target_rel/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level2-rel-open-complete":       {"link2/link1_rel/../target", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-open-complete1":      {"link2/link1_rel/../target", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-open-complete2":      {"link2/link1_rel/../target/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level2-rel-open-complete3":      {"link2/link1_rel/../target//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level2-rel-open-partial":        {"link2/link1_rel/../target/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level2-rel-open-partial-dotdot": {"link2/link1_rel/../target/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level3-abs-complete":            {"link3/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level3-abs-complete1":           {"link3/target_abs", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level3-abs-complete2":           {"link3/target_abs/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level3-abs-complete3":           {"link3/target_abs//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level3-abs-partial":             {"link3/target_abs/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level3-abs-partial-dotdot":      {"link3/target_abs/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
-		"nonlexical-level3-rel-complete":            {"link3/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level3-rel-complete1":           {"link3/target_rel", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level3-rel-complete2":           {"link3/target_rel/", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
+		"nonlexical-level3-rel-complete3":           {"link3/target_rel//", openResult{handlePath: "/target", fileType: unix.S_IFDIR}},
 		"nonlexical-level3-rel-partial":             {"link3/target_rel/foo", openResult{err: unix.ENOENT}},
 		"nonlexical-level3-rel-partial-dotdot":      {"link3/target_rel/../target/foo/bar/../baz", openResult{err: unix.ENOENT}},
 		// Partial lookups due to hitting a non-directory.
-		"partial-nondir-dot":             {"b/c/file/.", openResult{err: unix.ENOENT}},
-		"partial-nondir-dotdot1":         {"b/c/file/..", openResult{err: unix.ENOENT}},
-		"partial-nondir-dotdot2":         {"b/c/file/../foo/bar", openResult{err: unix.ENOENT}},
-		"partial-nondir-symlink-dot":     {"b-file/.", openResult{err: unix.ENOENT}},
-		"partial-nondir-symlink-dotdot1": {"b-file/..", openResult{err: unix.ENOENT}},
-		"partial-nondir-symlink-dotdot2": {"b-file/../foo/bar", openResult{err: unix.ENOENT}},
-		"partial-fifo-dot":               {"b/fifo/.", openResult{err: unix.ENOENT}},
-		"partial-fifo-dotdot1":           {"b/fifo/..", openResult{err: unix.ENOENT}},
-		"partial-fifo-dotdot2":           {"b/fifo/../foo/bar", openResult{err: unix.ENOENT}},
-		"partial-sock-dot":               {"b/sock/.", openResult{err: unix.ENOENT}},
-		"partial-sock-dotdot1":           {"b/sock/..", openResult{err: unix.ENOENT}},
-		"partial-sock-dotdot2":           {"b/sock/../foo/bar", openResult{err: unix.ENOENT}},
+		"partial-nondir-slash1":          {"b/c/file/", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-slash2":          {"b/c/file//", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-dot":             {"b/c/file/.", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-dotdot1":         {"b/c/file/..", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-dotdot2":         {"b/c/file/../foo/bar", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-symlink-slash1":  {"b-file/", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-symlink-slash2":  {"b-file//", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-symlink-dot":     {"b-file/.", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-symlink-dotdot1": {"b-file/..", openResult{err: unix.ENOTDIR}},
+		"partial-nondir-symlink-dotdot2": {"b-file/../foo/bar", openResult{err: unix.ENOTDIR}},
+		"partial-fifo-slash1":            {"b/fifo/", openResult{err: unix.ENOTDIR}},
+		"partial-fifo-slash2":            {"b/fifo//", openResult{err: unix.ENOTDIR}},
+		"partial-fifo-dot":               {"b/fifo/.", openResult{err: unix.ENOTDIR}},
+		"partial-fifo-dotdot1":           {"b/fifo/..", openResult{err: unix.ENOTDIR}},
+		"partial-fifo-dotdot2":           {"b/fifo/../foo/bar", openResult{err: unix.ENOTDIR}},
+		"partial-sock-slash1":            {"b/sock/", openResult{err: unix.ENOTDIR}},
+		"partial-sock-slash2":            {"b/sock//", openResult{err: unix.ENOTDIR}},
+		"partial-sock-dot":               {"b/sock/.", openResult{err: unix.ENOTDIR}},
+		"partial-sock-dotdot1":           {"b/sock/..", openResult{err: unix.ENOTDIR}},
+		"partial-sock-dotdot2":           {"b/sock/../foo/bar", openResult{err: unix.ENOTDIR}},
 		// Dangling symlinks are treated as though they are non-existent.
 		"dangling1-inroot-trailing":       {"a-fake1", openResult{err: unix.ENOENT}},
 		"dangling1-inroot-partial":        {"a-fake1/foo", openResult{err: unix.ENOENT}},
@@ -296,12 +329,12 @@ func testOpenInRoot(t *testing.T, openInRootFn openInRootFunc) {
 		"deep-dangling4":           {"dangling/d/e", openResult{err: unix.ENOENT}},
 		"deep-dangling5":           {"dangling/e", openResult{err: unix.ENOENT}},
 		"deep-dangling6":           {"dangling/g", openResult{err: unix.ENOENT}},
-		"deep-dangling-fileasdir1": {"dangling-file/a", openResult{err: unix.ENOENT}},
-		"deep-dangling-fileasdir2": {"dangling-file/b/c", openResult{err: unix.ENOENT}},
-		"deep-dangling-fileasdir3": {"dangling-file/c", openResult{err: unix.ENOENT}},
-		"deep-dangling-fileasdir4": {"dangling-file/d/e", openResult{err: unix.ENOENT}},
-		"deep-dangling-fileasdir5": {"dangling-file/e", openResult{err: unix.ENOENT}},
-		"deep-dangling-fileasdir6": {"dangling-file/g", openResult{err: unix.ENOENT}},
+		"deep-dangling-fileasdir1": {"dangling-file/a", openResult{err: unix.ENOTDIR}},
+		"deep-dangling-fileasdir2": {"dangling-file/b/c", openResult{err: unix.ENOTDIR}},
+		"deep-dangling-fileasdir3": {"dangling-file/c", openResult{err: unix.ENOTDIR}},
+		"deep-dangling-fileasdir4": {"dangling-file/d/e", openResult{err: unix.ENOTDIR}},
+		"deep-dangling-fileasdir5": {"dangling-file/e", openResult{err: unix.ENOTDIR}},
+		"deep-dangling-fileasdir6": {"dangling-file/g", openResult{err: unix.ENOTDIR}},
 		// Symlink loops.
 		"loop":        {"loop/link", openResult{err: unix.ELOOP}},
 		"loop-basic1": {"loop/basic-loop1", openResult{err: unix.ELOOP}},
@@ -364,12 +397,12 @@ func TestOpenInRoot_BadInode(t *testing.T) {
 			"char-trailing": {"foo/whiteout", openResult{handlePath: "/foo/whiteout", fileType: unix.S_IFCHR}},
 			"blk-trailing":  {"foo/whiteout-blk", openResult{handlePath: "/foo/whiteout-blk", fileType: unix.S_IFBLK}},
 			// Partial lookups due to hitting a non-directory.
-			"char-dot":     {"foo/whiteout/.", openResult{err: unix.ENOENT}},
-			"char-dotdot1": {"foo/whiteout/..", openResult{err: unix.ENOENT}},
-			"char-dotdot2": {"foo/whiteout/../foo/bar", openResult{err: unix.ENOENT}},
-			"blk-dot":      {"foo/whiteout-blk/.", openResult{err: unix.ENOENT}},
-			"blk-dotdot1":  {"foo/whiteout-blk/..", openResult{err: unix.ENOENT}},
-			"blk-dotdot2":  {"foo/whiteout-blk/../foo/bar", openResult{err: unix.ENOENT}},
+			"char-dot":     {"foo/whiteout/.", openResult{err: unix.ENOTDIR}},
+			"char-dotdot1": {"foo/whiteout/..", openResult{err: unix.ENOTDIR}},
+			"char-dotdot2": {"foo/whiteout/../foo/bar", openResult{err: unix.ENOTDIR}},
+			"blk-dot":      {"foo/whiteout-blk/.", openResult{err: unix.ENOTDIR}},
+			"blk-dotdot1":  {"foo/whiteout-blk/..", openResult{err: unix.ENOTDIR}},
+			"blk-dotdot2":  {"foo/whiteout-blk/../foo/bar", openResult{err: unix.ENOTDIR}},
 		} {
 			test := test // copy iterator
 			// Update the handlePath to be inside our root.

diff --git a/openat2_linux.go b/openat2_linux.go
@@ -87,6 +87,17 @@ func openat2File(dir *os.File, path string, how *unix.OpenHow) (*os.File, error)
 	return nil, &os.PathError{Op: "openat2", Path: fullPath, Err: errPossibleAttack}
 }
 
+func lookupOpenat2(root *os.File, unsafePath string, partial bool) (*os.File, string, error) {
+	if !partial {
+		file, err := openat2File(root, unsafePath, &unix.OpenHow{
+			Flags:   unix.O_PATH | unix.O_CLOEXEC,
+			Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS,
+		})
+		return file, "", err
+	}
+	return partialLookupOpenat2(root, unsafePath)
+}
+
 // partialLookupOpenat2 is an alternative implementation of
 // partialLookupInRoot, using openat2(RESOLVE_IN_ROOT) to more safely get a
 // handle to the deepest existing child of the requested path within the root.
@@ -95,6 +106,7 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
 
 	unsafePath = filepath.ToSlash(unsafePath) // noop
 	endIdx := len(unsafePath)
+	var lastError error
 	for endIdx > 0 {
 		subpath := unsafePath[:endIdx]
 
@@ -108,11 +120,12 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
 				endIdx += 1
 			}
 			// We found a subpath!
-			return handle, unsafePath[endIdx:], nil
+			return handle, unsafePath[endIdx:], lastError
 		}
 		if errors.Is(err, unix.ENOENT) || errors.Is(err, unix.ENOTDIR) {
 			// That path doesn't exist, let's try the next directory up.
 			endIdx = strings.LastIndexByte(subpath, '/')
+			lastError = err
 			continue
 		}
 		return nil, "", fmt.Errorf("open subpath: %w", err)
@@ -124,5 +137,5 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
 	if err != nil {
 		return nil, "", err
 	}
-	return rootClone, unsafePath, nil
+	return rootClone, unsafePath, lastError
 }
diff --git a/procfs_linux.go b/procfs_linux.go
@@ -10,7 +10,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"path/filepath"
 	"runtime"
 	"strconv"
 	"sync"
@@ -160,7 +159,7 @@ func clonePrivateProcMount() (_ *os.File, Err error) {
 }
 
 func privateProcRoot() (*os.File, error) {
-	if !hasNewMountApi() {
+	if !hasNewMountApi() || testingForceGetProcRootUnsafe() {
 		return nil, fmt.Errorf("new mount api: %w", unix.ENOTSUP)
 	}
 	// Try to create a new procfs mount from scratch if we can. This ensures we
@@ -199,7 +198,7 @@ func unsafeHostProcRoot() (_ *os.File, Err error) {
 
 func doGetProcRoot() (*os.File, error) {
 	procRoot, err := privateProcRoot()
-	if err != nil || testingForceGetProcRootUnsafe(procRoot) {
+	if err != nil {
 		// Fall back to using a /proc handle if making a private mount failed.
 		// If we have openat2, at least we can avoid some kinds of over-mount
 		// attacks, but without openat2 there's not much we can do.
@@ -286,14 +285,14 @@ func procThreadSelf(procRoot *os.File, subpath string) (_ *os.File, _ procThread
 		//       procSelfFdReadlink to clean up the returned f.Name() if we use
 		//       RESOLVE_IN_ROOT (which would lead to an infinite recursion).
 		handle, err = openat2File(procRoot, threadSelf+subpath, &unix.OpenHow{
-			Flags:   unix.O_PATH | unix.O_CLOEXEC,
+			Flags:   unix.O_PATH | unix.O_NOFOLLOW | unix.O_CLOEXEC,
 			Resolve: unix.RESOLVE_BENEATH | unix.RESOLVE_NO_XDEV | unix.RESOLVE_NO_MAGICLINKS,
 		})
 		if err != nil {
 			return nil, nil, fmt.Errorf("%w: %w", errUnsafeProcfs, err)
 		}
 	} else {
-		handle, err = openatFile(procRoot, threadSelf+subpath, unix.O_PATH|unix.O_CLOEXEC, 0)
+		handle, err = openatFile(procRoot, threadSelf+subpath, unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
 		if err != nil {
 			return nil, nil, fmt.Errorf("%w: %w", errUnsafeProcfs, err)
 		}
@@ -333,10 +332,10 @@ func hasStatxMountId() bool {
 	return hasStatxMountIdBool
 }
 
-func checkSymlinkOvermount(dir *os.File, path string) error {
+func getMountId(dir *os.File, path string) (uint64, error) {
 	// If we don't have statx(STATX_MNT_ID*) support, we can't do anything.
 	if !hasStatxMountId() {
-		return nil
+		return 0, nil
 	}
 
 	var (
@@ -346,31 +345,29 @@ func checkSymlinkOvermount(dir *os.File, path string) error {
 		wantStxMask uint32 = unix.STATX_MNT_ID_UNIQUE | unix.STATX_MNT_ID
 	)
 
-	// Get the mntId of our procfs handle.
-	err := unix.Statx(int(dir.Fd()), "", unix.AT_EMPTY_PATH, int(wantStxMask), &stx)
-	if err != nil {
-		return &os.PathError{Op: "statx", Path: dir.Name(), Err: err}
-	}
+	err := unix.Statx(int(dir.Fd()), path, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW, int(wantStxMask), &stx)
 	if stx.Mask&wantStxMask == 0 {
 		// It's not a kernel limitation, for some reason we couldn't get a
 		// mount ID. Assume it's some kind of attack.
-		return fmt.Errorf("%w: could not get mnt id of dir %s", errUnsafeProcfs, dir.Name())
+		err = fmt.Errorf("%w: could not get mount id", errUnsafeProcfs)
+	}
+	if err != nil {
+		return 0, &os.PathError{Op: "statx(STATX_MNT_ID_...)", Path: dir.Name() + "/" + path, Err: err}
 	}
-	expectedMountId := stx.Mnt_id
+	return stx.Mnt_id, nil
+}
 
-	// Get the mntId of the target symlink.
-	stx = unix.Statx_t{}
-	err = unix.Statx(int(dir.Fd()), path, unix.AT_SYMLINK_NOFOLLOW, int(wantStxMask), &stx)
+func checkSymlinkOvermount(procRoot *os.File, dir *os.File, path string) error {
+	// Get the mntId of our procfs handle.
+	expectedMountId, err := getMountId(procRoot, "")
 	if err != nil {
-		return &os.PathError{Op: "statx", Path: dir.Name() + "/" + path, Err: err}
+		return err
 	}
-	if stx.Mask&wantStxMask == 0 {
-		// It's not a kernel limitation, for some reason we couldn't get a
-		// mount ID. Assume it's some kind of attack.
-		return fmt.Errorf("%w: could not get mnt id of symlink %s", errUnsafeProcfs, path)
+	// Get the mntId of the target magic-link.
+	gotMountId, err := getMountId(dir, path)
+	if err != nil {
+		return err
 	}
-	gotMountId := stx.Mnt_id
-
 	// As long as the directory mount is alive, even with wrapping mount IDs,
 	// we would expect to see a different mount ID here. (Of course, if we're
 	// using unsafeHostProcRoot() then an attaker could change this after we
@@ -381,37 +378,33 @@ func checkSymlinkOvermount(dir *os.File, path string) error {
 	return nil
 }
 
-func doProcSelfMagiclink[T any](procRoot *os.File, subPath string, fn func(procDirHandle *os.File, base string) (T, error)) (T, error) {
-	// We cannot operate on the magic-link directly with a handle, we need to
-	// create a handle to the parent of the magic-link and then do
-	// single-component operations on it.
-	dir, base := filepath.Dir(subPath), filepath.Base(subPath)
-
-	procDirHandle, closer, err := procThreadSelf(procRoot, dir)
+func doRawProcSelfFdReadlink(procRoot *os.File, fd int) (string, error) {
+	fdPath := fmt.Sprintf("fd/%d", fd)
+	procFdLink, closer, err := procThreadSelf(procRoot, fdPath)
 	if err != nil {
-		return *new(T), fmt.Errorf("get safe /proc/thread-self/%s handle: %w", dir, err)
+		return "", fmt.Errorf("get safe /proc/thread-self/%s handle: %w", fdPath, err)
 	}
-	defer procDirHandle.Close()
+	defer procFdLink.Close()
 	defer closer()
 
-	// Try to detect if there is a mount on top of the symlink we are about to
-	// read. If we are using unsafeHostProcRoot(), this could change after we
-	// check it (and there's nothing we can do about that) but for
-	// privateProcRoot() this should be guaranteed to be safe (at least since
-	// Linux 5.12[1], when anonymous mount namespaces were completely isolated
-	// from external mounts including mount propagation events).
+	// Try to detect if there is a mount on top of the magic-link. Since we use the handle directly
+	// provide to the closure. If the closure uses the handle directly, this
+	// should be safe in general (a mount on top of the path afterwards would
+	// not affect the handle itself) and will definitely be safe if we are
+	// using privateProcRoot() (at least since Linux 5.12[1], when anonymous
+	// mount namespaces were completely isolated from external mounts including
+	// mount propagation events).
 	//
 	// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
 	// onto targets that reside on shared mounts").
-	if err := checkSymlinkOvermount(procDirHandle, base); err != nil {
-		return *new(T), fmt.Errorf("check safety of %s proc magiclink: %w", subPath, err)
+	if err := checkSymlinkOvermount(procRoot, procFdLink, ""); err != nil {
+		return "", fmt.Errorf("check safety of /proc/thread-self/fd/%d magiclink: %w", fd, err)
 	}
-	return fn(procDirHandle, base)
-}
 
-func doRawProcSelfFdReadlink(procRoot *os.File, fd int) (string, error) {
-	fdPath := fmt.Sprintf("fd/%d", fd)
-	return doProcSelfMagiclink(procRoot, fdPath, readlinkatFile)
+	// readlinkat implies AT_EMPTY_PATH since Linux 2.6.39. See Linux commit
+	// 65cfc6722361 ("readlinkat(), fchownat() and fstatat() with empty
+	// relative pathnames").
+	return readlinkatFile(procFdLink, "")
 }
 
 func rawProcSelfFdReadlink(fd int) (string, error) {

diff --git a/procfs_linux_test.go b/procfs_linux_test.go
@@ -136,17 +136,30 @@ func testProcOvermountSubdir(t *testing.T, procRootFn procRootFunc, expectOvermo
 			symlinkOvermountErr = nil
 		}
 
-		// Check that /proc/self/exe and .
-		procThreadSelf, closer, err := procThreadSelf(procRoot, ".")
+		procSelf, closer, err := procThreadSelf(procRoot, ".")
 		require.NoError(t, err)
-		defer procThreadSelf.Close()
+		defer procSelf.Close()
 		defer closer()
 
+		// Open these paths directly to emulate a non-openat2 handle that
+		// didn't detect a bind-mount to check that checkSymlinkOvermount works
+		// properly for AT_EMPTY_PATH checks as well.
+		procCwd, err := openatFile(procSelf, "cwd", unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
+		require.NoError(t, err)
+		defer procCwd.Close()
+		procExe, err := openatFile(procSelf, "exe", unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
+		require.NoError(t, err)
+		defer procExe.Close()
+
 		// no overmount
-		err = checkSymlinkOvermount(procThreadSelf, "cwd")
+		err = checkSymlinkOvermount(procRoot, procCwd, "")
+		assert.NoError(t, err, "checking /proc/self/cwd with no overmount should succeed")
+		err = checkSymlinkOvermount(procRoot, procSelf, "cwd")
 		assert.NoError(t, err, "checking /proc/self/cwd with no overmount should succeed")
 		// basic overmount
-		err = checkSymlinkOvermount(procThreadSelf, "exe")
+		err = checkSymlinkOvermount(procRoot, procExe, "")
+		assert.ErrorIs(t, err, symlinkOvermountErr, "unexpected /proc/self/exe overmount result")
+		err = checkSymlinkOvermount(procRoot, procSelf, "exe")
 		assert.ErrorIs(t, err, symlinkOvermountErr, "unexpected /proc/self/exe overmount result")
 
 		// fd no overmount

diff --git a/testing_mocks_linux.go b/testing_mocks_linux.go
@@ -42,9 +42,9 @@ func testingForcePrivateProcRootOpenTreeAtRecursive(f *os.File) bool {
 		testingCheckClose(*testingForceGetProcRoot >= forceGetProcRootOpenTreeAtRecursive, f)
 }
 
-func testingForceGetProcRootUnsafe(f *os.File) bool {
+func testingForceGetProcRootUnsafe() bool {
 	return testing.Testing() && testingForceGetProcRoot != nil &&
-		testingCheckClose(*testingForceGetProcRoot >= forceGetProcRootUnsafe, f)
+		*testingForceGetProcRoot >= forceGetProcRootUnsafe
 }
 
 type forceProcThreadSelfLevel int

diff --git a/util_linux_test.go b/util_linux_test.go
@@ -89,29 +89,28 @@ func hasRenameExchange() bool {
 }
 
 func doRenameExchangeLoop(pauseCh chan struct{}, exitCh <-chan struct{}, dir *os.File, pathA, pathB string) {
-	var swapped bool
-	swap := func() {
-		_ = unix.Renameat2(int(dir.Fd()), pathA, int(dir.Fd()), pathB, unix.RENAME_EXCHANGE)
-		//unix.Sync()
-		swapped = !swapped
-	}
-
 	for {
 		select {
 		case <-exitCh:
 			return
 		case <-pauseCh:
-			if swapped {
-				swap()
-			}
 			// Wait for caller to unpause us.
 			select {
 			case pauseCh <- struct{}{}:
 			case <-exitCh:
 				return
 			}
 		default:
-			swap()
+			// Do the swap twice so that we only pause when we are in a
+			// "correct" state.
+			for i := 0; i < 2; i++ {
+				err := unix.Renameat2(int(dir.Fd()), pathA, int(dir.Fd()), pathB, unix.RENAME_EXCHANGE)
+				if err != nil && !errors.Is(err, unix.EBADF) {
+					// Should never happen, and if it does we will potentially
+					// enter a bad filesystem state if we get paused.
+					panic(fmt.Sprintf("renameat2([%d]%q, %q, ..., %q, RENAME_EXCHANGE) = %v", int(dir.Fd()), dir.Name(), pathA, pathB, err))
+				}
+			}
 		}
 	}
 }