go: x/sys/unix: Setfsuid and Setfsgid have wrong return values

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

go version go1.13.1 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/s/go/bin"
GOCACHE="/home/s/.cache/go-build"
GOENV="/home/s/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/s/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build184928266=/tmp/go-build -gno-record-gcc-switches"

What did you do?

According to the documentation, both setfsuid() and setfsgid() should return the previous filesystem user or group ID which is not the case in go’s syscall package. The signatures in syscall package are (for all architectures):

Setfsuid(uid int) (err error)
Setfsgid(gid int) (err error)

What did you expect to see?

Correct ones would be

Setfsuid(uid int) (prev int, err error)
Setfsgid(uid int) (prev int, err error)

Right now there’s no way to determine if the call succeeded or not as the err value will always be nil both on successful call and on fail.

Though it can be fixed just by adding an extra return value to the function signatures in syscall_linux_%arch%.go, it would be a breaking change. Another way is to introduce another version of these functions with correct signatures. Linux uses that approach for its epoll functionality ( epoll_create(2) and epoll_create1(2)). But to be honest, it’s even worse than breaking the backwards compatibility in my opinion, as the code will become a total mess after a few ‘fixes’ like that one.

What did you see instead?

Pretty much meaningless return value after a call that didn’t fail due to lack of CAP_SETUID :c

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 16 (8 by maintainers)

Most upvoted comments

Please wait for @ianlancetaylor to approve any name. Ideally it should match some existing naming convention precedent. That may not be possible, in which case getting the precedent nice for such future cases is worth getting more people’s thoughts on.