From fc45eb388d3693e35b1aa97cb553d2a5083c8b2f Mon Sep 17 00:00:00 2001 From: miller Date: Tue, 7 Mar 2023 15:15:10 +0000 Subject: [PATCH] syscall: avoid race in plan9 while syncing Chdir across goroutines Because each M in Plan 9 runs in a separate OS process with its own current working directory, a Chdir call in one goroutine needs to be propagated to other goroutines before a subsequent syscall with a local pathname (see #9428). This is done by function syscall.Fixwd, but there is still a race if a goroutine is preempted and rescheduled on a different M between calling Fixwd and executing the syscall which it protects. By locking the goroutine to its OS thread from the start of Fixwd to the end of the protected syscall, this race can be prevented. Fixes #58802. Change-Id: I89c0e43ef4544b5bfb5db7d2158f13f24b42e1f6 Reviewed-on: https://go-review.googlesource.com/c/go/+/474055 Reviewed-by: Bryan Mills Auto-Submit: Ian Lance Taylor TryBot-Result: Gopher Robot Run-TryBot: Ian Lance Taylor Reviewed-by: Ian Lance Taylor --- src/syscall/pwd_plan9.go | 26 ++++++++++++++++++++++---- src/syscall/syscall_plan9.go | 33 +++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/syscall/pwd_plan9.go b/src/syscall/pwd_plan9.go index 1deeaa9061..28e99565ee 100644 --- a/src/syscall/pwd_plan9.go +++ b/src/syscall/pwd_plan9.go @@ -11,7 +11,10 @@ package syscall -import "sync" +import ( + "runtime" + "sync" +) var ( wdmu sync.Mutex // guards following @@ -19,6 +22,12 @@ var ( wdStr string ) +// Ensure current working directory seen by this goroutine matches +// the most recent Chdir called in any goroutine. It's called internally +// before executing any syscall which uses a relative pathname. Must +// be called with the goroutine locked to the OS thread, to prevent +// rescheduling on a different thread (potentially with a different +// working directory) before the syscall is executed. func Fixwd() { wdmu.Lock() defer wdmu.Unlock() @@ -39,13 +48,17 @@ func fixwdLocked() { } } -func fixwd(paths ...string) { +// If any of the paths is relative, call Fixwd and return true +// (locked to OS thread). Otherwise return false. +func fixwd(paths ...string) bool { for _, path := range paths { if path != "" && path[0] != '/' && path[0] != '#' { + runtime.LockOSThread() Fixwd() - return + return true } } + return false } // goroutine-specific getwd @@ -75,10 +88,15 @@ func Getwd() (wd string, err error) { } func Chdir(path string) error { - fixwd(path) + // If Chdir is to a relative path, sync working dir first + if fixwd(path) { + defer runtime.UnlockOSThread() + } wdmu.Lock() defer wdmu.Unlock() + runtime.LockOSThread() + defer runtime.UnlockOSThread() if err := chdir(path); err != nil { return err } diff --git a/src/syscall/syscall_plan9.go b/src/syscall/syscall_plan9.go index ca286c8c97..92414709ef 100644 --- a/src/syscall/syscall_plan9.go +++ b/src/syscall/syscall_plan9.go @@ -13,6 +13,7 @@ package syscall import ( "internal/oserror" + "runtime" "unsafe" ) @@ -301,7 +302,9 @@ func Await(w *Waitmsg) (err error) { } func Unmount(name, old string) (err error) { - fixwd(name, old) + if fixwd(name, old) { + defer runtime.UnlockOSThread() + } oldp, err := BytePtrFromString(old) if err != nil { return err @@ -384,49 +387,63 @@ func Getgroups() (gids []int, err error) { //sys open(path string, mode int) (fd int, err error) func Open(path string, mode int) (fd int, err error) { - fixwd(path) + if fixwd(path) { + defer runtime.UnlockOSThread() + } return open(path, mode) } //sys create(path string, mode int, perm uint32) (fd int, err error) func Create(path string, mode int, perm uint32) (fd int, err error) { - fixwd(path) + if fixwd(path) { + defer runtime.UnlockOSThread() + } return create(path, mode, perm) } //sys remove(path string) (err error) func Remove(path string) error { - fixwd(path) + if fixwd(path) { + defer runtime.UnlockOSThread() + } return remove(path) } //sys stat(path string, edir []byte) (n int, err error) func Stat(path string, edir []byte) (n int, err error) { - fixwd(path) + if fixwd(path) { + defer runtime.UnlockOSThread() + } return stat(path, edir) } //sys bind(name string, old string, flag int) (err error) func Bind(name string, old string, flag int) (err error) { - fixwd(name, old) + if fixwd(name, old) { + defer runtime.UnlockOSThread() + } return bind(name, old, flag) } //sys mount(fd int, afd int, old string, flag int, aname string) (err error) func Mount(fd int, afd int, old string, flag int, aname string) (err error) { - fixwd(old) + if fixwd(old) { + defer runtime.UnlockOSThread() + } return mount(fd, afd, old, flag, aname) } //sys wstat(path string, edir []byte) (err error) func Wstat(path string, edir []byte) (err error) { - fixwd(path) + if fixwd(path) { + defer runtime.UnlockOSThread() + } return wstat(path, edir) } -- 2.50.0