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

cmd/go: go fmt aborts with operation not permitted when using mounted vmware shared folders (hgfs) #60225

Closed
mdepot opened this issue May 16, 2023 · 4 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdepot
Copy link

mdepot commented May 16, 2023

What version of Go are you using (go version)?

$ go version
go version go1.20.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, version 1.20.4 is the latest release as of this writing

What operating system and processor architecture are you using (go env)?

Rocky Linux 8.7 (amd64) in a VM, running on top of VMware Workstation 17 Pro, on Windows 10

go env:
$ go env
GOARCH="amd64"
GOCACHE="/home/myuser/.cache/go-build"
GOENV="/home/myuser/.config/go/env"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOMODCACHE="/home/myuser/go/pkg/mod"
GOOS="linux"
GOPATH="/home/myuser/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVERSION="go1.20.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/mnt/hgfs/vmshare/golang/myapp/go.mod"
CGO_CFLAGS="-O2 -g"
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2445446007=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ ll
total 52
drwxrwxrwx. 1 root root  4096 May 16 08:56 cmd
-rwxrwxrwx. 1 root root   840 May 15 16:01 go.mod
-rwxrwxrwx. 1 root root 47337 May 15 16:01 go.sum
-rwxrwxrwx. 1 root root     0 May 15 16:01 LICENSE
-rwxrwxrwx. 1 root root   152 May 15 16:01 main.go

$ sudo chown myuser *.go
[sudo] password for myuser: 

$ sudo chmod 640 *.go

$ ll
total 52
drwxrwxrwx. 1 root root  4096 May 16 08:56 cmd
-rwxrwxrwx. 1 root root   840 May 15 16:01 go.mod
-rwxrwxrwx. 1 root root 47337 May 15 16:01 go.sum
-rwxrwxrwx. 1 root root     0 May 15 16:01 LICENSE
-rwxrwxrwx. 1 root root   152 May 15 16:01 main.go

$ go fmt main.go
main.go
chmod ./main.go.2356257763: operation not permitted
exit status 2

Comments

The go fmt command above appears to create a temporary file in the current dir, and then apparently runs chmod on
that file. It would seem the chmod should not be needed to gain access to the file by go fmt, so I presume the
purpose of the chmod was actually to impose access restriction to the temp file by other users/processes during the
formatting operation.

In my case the file system of the working dir is a mounted vmware shared folder. This is a windows directory that
is shared with the linux VM by means of vmware tools. From the perspective of linux, the file permissions on such a
mount will always be 777 and owned by root, as demonstrated by the ineffective chown and chmod commands above. When
the go fmt command tries and fails to do a chmod on the temp file, it completely aborts the code formatting
operation.

Handling for this should be improved so that code formatting can still work when using various (less capable)
mounted file system types. It may make sense to check the permissions of the temp file, and only call chmod if the
permissions on the temp file are actually less restrictive than the permissions on source file for example. Another
option might be to attempt the chmod but not completely abort on failure, although the potential security
implications of that option should be considered.

@heschi heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 16, 2023
@heschi
Copy link
Contributor

heschi commented May 16, 2023

cc @griesemer @mvdan

@heschi heschi added this to the Backlog milestone May 16, 2023
@ianlancetaylor
Copy link
Contributor

The chmod isn't to protect the file. It's simply that go fmt creates a backup file before rewriting the original file. It creates the backup file by creating a temporary file and then calling chmod to set the permissions of the temporary file to the permissions of the original file. Then if rewriting the original file fails, it renames the backup file to the original file, so that the file is not lost.

It would be slightly more efficient and slightly more secure to create the backup file with the correct permissions in the first place, but the code just calls os.CreateTemp which doesn't have a way to specify the permissions. If we did that, somehow, then I assume that this problem would go away. We could also skip the chmod if the permissions happen to be exactly the same as the temporary file (0o600) but that is not particularly likely and is not the case in your example.

@gopherbot
Copy link

Change https://go.dev/cl/495316 mentions this issue: cmd/gofmt: don't clobber original file on permission error

@mdepot mdepot changed the title cmd/go: go fmt aborts with operation not permitted when using mounted vmware shared folders file system cmd/go: go fmt aborts with operation not permitted when using mounted vmware shared folders (hgfs) May 17, 2023
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 17, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 May 17, 2023
@gopherbot
Copy link

Change https://go.dev/cl/496155 mentions this issue: cmd/gofmt: fix a data race in TestPermissions

gopherbot pushed a commit that referenced this issue May 18, 2023
The asynchronous call to processFile is synchronized by the call to
GetExitCode. We can't safely access errBuf until then, because
processFile may still be writing to it.

This is diagnosed by 'go test -race cmd/gofmt', but only the
darwin-amd64-race builder caught it because the other "-race" builders
apparently all run as root (see #10719).

Updates #60225.

Change-Id: Ie66bb4e47429ece81043d6425f26953b7bb26002
Reviewed-on: https://go-review.googlesource.com/c/go/+/496155
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants