From da9a13289bfec9255fd837432db5bff43f19e05b Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Wed, 2 Sep 2026 11:54:24 -0700 Subject: [PATCH] Fix inode path result pointers across stack relocation The old request stored the result address as an integer, which Go could not update when the stack moved. Use a typed pointer to prevent stale addresses and empty lookup results. Keep Go 1.16 compatibility. --- c_defs_linux.go | 1 + scoutfs.go | 19 +++++++-- scoutfs_linux_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++ scoutfs_test.go | 85 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 scoutfs_linux_test.go create mode 100644 scoutfs_test.go diff --git a/c_defs_linux.go b/c_defs_linux.go index 561616c..0d4af2f 100644 --- a/c_defs_linux.go +++ b/c_defs_linux.go @@ -1,4 +1,5 @@ //go:build ignore +// +build ignore // Copyright (c) 2018 Versity Software, Inc. // diff --git a/scoutfs.go b/scoutfs.go index 74ba189..ac9d4e6 100644 --- a/scoutfs.go +++ b/scoutfs.go @@ -217,6 +217,17 @@ func FSetAttrMore(f *os.File, version, size, flags uint64, ctime time.Time, crti return err } +// inoPathRequest matches the 64-bit ioctl layout while keeping the result +// pointer visible to Go's garbage collector and stack relocation. +type inoPathRequest struct { + Ino uint64 + Dir_ino uint64 + Dir_pos uint64 + Result_ptr *inoPathResult + Result_bytes uint16 + _ [6]uint8 +} + type inoPathResult struct { DirIno uint64 DirPos uint64 @@ -230,9 +241,9 @@ type inoPathResult struct { // (usually just the base mount point directory) func InoToPath(dirfd *os.File, ino uint64) (string, error) { var res inoPathResult - ip := inoPath{ + ip := inoPathRequest{ Ino: ino, - Result_ptr: uint64(uintptr(unsafe.Pointer(&res))), + Result_ptr: &res, Result_bytes: uint16(unsafe.Sizeof(res)), } @@ -251,9 +262,9 @@ func InoToPath(dirfd *os.File, ino uint64) (string, error) { // (usually just the base mount point directory) func InoToPaths(dirfd *os.File, ino uint64) ([]string, error) { var res inoPathResult - ip := inoPath{ + ip := inoPathRequest{ Ino: ino, - Result_ptr: uint64(uintptr(unsafe.Pointer(&res))), + Result_ptr: &res, Result_bytes: uint16(unsafe.Sizeof(res)), } diff --git a/scoutfs_linux_test.go b/scoutfs_linux_test.go new file mode 100644 index 0000000..76d14a7 --- /dev/null +++ b/scoutfs_linux_test.go @@ -0,0 +1,90 @@ +package scoutfs + +import ( + "errors" + "os" + "path/filepath" + "reflect" + "runtime" + "sort" + "strings" + "syscall" + "testing" +) + +func TestInodePathsOnScoutFS(t *testing.T) { + root := os.Getenv("SCOUTFS_TEST_MOUNT") + if root == "" { + t.Skip("set SCOUTFS_TEST_MOUNT to a writable ScoutFS mount root") + } + root, err := filepath.Abs(root) + if err != nil { + t.Fatal(err) + } + dirfd, err := os.Open(root) + if err != nil { + t.Fatal(err) + } + defer dirfd.Close() + dir, err := os.MkdirTemp(root, "inode-path-test-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + name := filepath.Join(dir, strings.Repeat("a", 200)) + if err := os.WriteFile(name, []byte("path lookup"), 0600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "second-link") + if err := os.Link(name, link); err != nil { + t.Fatal(err) + } + fi, err := os.Stat(name) + if err != nil { + t.Fatal(err) + } + ino := fi.Sys().(*syscall.Stat_t).Ino + want := []string{filepath.Join(filepath.Base(dir), filepath.Base(name)), filepath.Join(filepath.Base(dir), "second-link")} + sort.Strings(want) + + stop := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + for { + select { + case <-stop: + return + default: + runtime.GC() + } + } + }() + defer func() { close(stop); <-done }() + for i := 0; i < 256; i++ { + path, err := InoToPath(dirfd, ino) + if err != nil || (path != want[0] && path != want[1]) { + t.Fatalf("InoToPath = %q, %v; want one of %v", path, err, want) + } + paths, err := InoToPaths(dirfd, ino) + if err != nil { + t.Fatal(err) + } + sort.Strings(paths) + if !reflect.DeepEqual(paths, want) { + t.Fatalf("InoToPaths = %v, want %v", paths, want) + } + } + if err := os.Remove(name); err != nil { + t.Fatal(err) + } + if err := os.Remove(link); err != nil { + t.Fatal(err) + } + if path, err := InoToPath(dirfd, ino); path != "" || !errors.Is(err, syscall.ENOENT) { + t.Fatalf("deleted InoToPath = %q, %v; want empty path and ENOENT", path, err) + } + if paths, err := InoToPaths(dirfd, ino); len(paths) != 0 || err != nil { + t.Fatalf("deleted InoToPaths = %v, %v; want no paths and no error", paths, err) + } +} diff --git a/scoutfs_test.go b/scoutfs_test.go new file mode 100644 index 0000000..4a28bcb --- /dev/null +++ b/scoutfs_test.go @@ -0,0 +1,85 @@ +package scoutfs + +import ( + "errors" + "os" + "runtime" + "syscall" + "testing" + "unsafe" +) + +func TestInoPathLayout(t *testing.T) { + var request inoPathRequest + var abi inoPath + var result inoPathResult + for name, layout := range map[string]struct{ got, want uintptr }{ + "request size": {unsafe.Sizeof(request), 40}, + "generated size": {unsafe.Sizeof(request), unsafe.Sizeof(abi)}, + "inode": {unsafe.Offsetof(request.Ino), unsafe.Offsetof(abi.Ino)}, + "directory": {unsafe.Offsetof(request.Dir_ino), unsafe.Offsetof(abi.Dir_ino)}, + "position": {unsafe.Offsetof(request.Dir_pos), unsafe.Offsetof(abi.Dir_pos)}, + "pointer ABI": {unsafe.Offsetof(request.Result_ptr), unsafe.Offsetof(abi.Result_ptr)}, + "length ABI": {unsafe.Offsetof(request.Result_bytes), unsafe.Offsetof(abi.Result_bytes)}, + "result pointer": {unsafe.Offsetof(request.Result_ptr), 24}, + "result bytes": {unsafe.Offsetof(request.Result_bytes), 32}, + "path size": {unsafe.Offsetof(result.PathSize), 16}, + "path data": {unsafe.Offsetof(result.Path), 24}, + "result size": {unsafe.Sizeof(result), 24 + pathmax}, + } { + if layout.got != layout.want { + t.Errorf("%s = %d, want %d", name, layout.got, layout.want) + } + } +} + +func TestInoToPathErrors(t *testing.T) { + f, err := os.Open(os.DevNull) + if err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + for i := 0; i < 32; i++ { + name, err := InoToPath(f, 11293) + if !errors.Is(err, syscall.EBADF) || name != "" { + t.Fatalf("InoToPath = %q, %v; want empty path and EBADF", name, err) + } + names, err := InoToPaths(f, 11293) + if !errors.Is(err, syscall.EBADF) || names != nil { + t.Fatalf("InoToPaths = %v, %v; want nil paths and EBADF", names, err) + } + } +} + +//go:noinline +func growInodePathTestStack(depth int) byte { + var pad [1024]byte + pad[depth%len(pad)] = byte(depth) + if depth > 0 { + pad[0] = growInodePathTestStack(depth - 1) + } + runtime.KeepAlive(&pad) + return pad[depth%len(pad)] +} + +//go:noinline +func fillInodePathTestResult(request *inoPathRequest) { + growInodePathTestStack(64) + request.Result_ptr.PathSize = 7 + copy(request.Result_ptr.Path[:], "test/f\x00") +} + +func TestInoPathResultSurvivesStackGrowth(t *testing.T) { + done := make(chan bool) + go func() { + var res inoPathResult + request := inoPathRequest{Ino: 11293, Result_ptr: &res, Result_bytes: uint16(unsafe.Sizeof(res))} + fillInodePathTestResult(&request) + done <- request.Result_ptr == &res && res.PathSize == 7 && string(res.Path[:7]) == "test/f\x00" + }() + if !<-done { + t.Fatal("request result pointer did not follow its buffer across stack growth") + } +}