Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: syscall: add Umask option to SysProcAttr for Unix-like platforms #56016

Open
corhere opened this issue Oct 3, 2022 · 0 comments
Open
Labels
Milestone

Comments

@corhere
Copy link

corhere commented Oct 3, 2022

A Go program on Linux, *BSD, AIX, Solaris or Darwin is currently able to control the working directory and root directory of new processes launched with os.StartProcess by setting the (os.ProcAttr).Dir and (syscall.SysProcAttr).Chroot fields, respectively. In contrast, there is no way to control the file-creation mode mask (umask) attribute of started processes without introducing a race condition or paying a performance penalty to start an extraneous process or thread. I propose extending the syscall.SysProcAttr structs on the aforementioned platforms to permit setting the child's umask.

type SysProcAttr struct {
	// [existing fields omitted...]
	SetUmask bool
	Umask    int  // Child's umask if SetUmask.
}

New processes inherit the umask value of the parent, so one might think they could control the umask by writing something like:

// DO NOT DO THIS
runtime.LockOSThread() // Does nothing to prevent the race condition below.
defer runtime.UnlockOSThread()
old := syscall.Umask(newUmask) // RACE CONDITION: modifies the umask attribute for the whole process.
defer syscall.Umask(old) // Ditto.
return os.StartProcess(...) // New process gets newUmask, maybe.

Because all threads of a Go process share the same working directory, root directory and umask attributes, any files created by other goroutines between the two syscall.Umask calls would be affected by the temporary umask change, and multiple goroutines interleaving temporary umask changes could result in an incorrect umask being restored.

The race conditions can be avoided by changing the umask in an execution context which does not share file system attributes with the threads of the Go process which goroutines execute on. The most portable method to do so requires performing an extra fork+exec (for example) though as of Go 1.10 a slightly less expensive alternative can be used on Linux which "only" costs one extra thread and the need to worry about #27505.

// Linux only:
go func() {
	runtime.LockOSThread()
	// This operation is irreversible so the thread cannot be reused for other goroutines.
	syscall.Unshare(syscall.CLONE_FS)
	syscall.Umask(newUmask) // Only affects the current goroutine, and any processes spawned from it.
	os.StartProcess(...)
	// Return without calling runtime.UnlockOSThread() so the thread terminates.
	// The runtime will have to spawn a new thread at some point to replace it.
}()

The extra process/thread would not be needed if the umask could be set between fork and exec, exactly like how the working directory and root directory are currently set when spawning child processes.

@gopherbot gopherbot added this to the Proposal milestone Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

2 participants