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

syscall: exec_linux.go not using cross architecture safe SYS_SETGROUPS #17092

Closed
celledge opened this issue Sep 13, 2016 · 7 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@celledge
Copy link

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

go 1.7.1

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

GOOS=linux
GOARCH=arm

What did you do?

groups = []uint32{20, 21, 22}
cmd.SysProcAttr.Credential = &syscall.Credential{Groups: groups}
cmd.Run()

Run strace on the process:
strace -e trace=setgroups -e trace=setgroups32 -f {executable}

What did you expect to see?

setgroups32(3, [20, 21, 22])

What did you see instead?

setgroups(3, [20, 0, 21])

Source of problem and possible fix

It appears that syscall/linux_exec.go line 217, the RawSyscall is using SYS_SETGROUPS which on linux/arm is the 16bit GID system call. Since golang always uses 32bit GIDs, this fails. I switched this statement to SYS_SETGROUPS32, and it worked fine on my linux/arm system.

Perhaps this RawSyscall should be replaced with syscall.setgroups so that the correct architecture dependent system call is used.

Please note that this is also true for the SYS_SETUID and SYS_SETGID syscalls just below this line as well. Those statements will operate correctly, unless a UID or GID >= 2^16 is used. In which case it will truncate the integer and use the wrong ID instead of giving an error. However, I do not see an architecture dependent function already implemented to replace those.

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Sep 13, 2016
@ianlancetaylor ianlancetaylor changed the title exec_linux.go not using cross architecture safe SYS_SETGROUPS syscall: exec_linux.go not using cross architecture safe SYS_SETGROUPS Sep 13, 2016
@ianlancetaylor
Copy link
Contributor

CC @LK4D4

@LK4D4
Copy link
Contributor

LK4D4 commented Sep 13, 2016

@ianlancetaylor This code was there before me :/ Looks pretty bad and I don't about policy in forkAndExecInChild - if it's possible to call functions instead of RawSyscall there. I hope there won't hack as with s390x SYS_CLONE :)

@ianlancetaylor
Copy link
Contributor

@LK4D4 Oh, sorry, I just assumed it was your code.

The child process in forkAndExecInChild can call any function, as long as it is nosplit and does not allocate any memory.

@LK4D4
Copy link
Contributor

LK4D4 commented Sep 13, 2016

Just to be sure, here is function for arm:
https://github.com/golang/go/blob/master/src/syscall/zsyscall_linux_arm.go#L1263
Also, there are no functions for setuid and setgid. They need to be created in something like exec_linux_arm.go :(

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@gopherbot
Copy link

CL https://golang.org/cl/31458 mentions this issue.

@celledge
Copy link
Author

celledge commented Nov 8, 2016

I went over the patch at https://golang.org/cl/31458
It does fix the SYS_SETGROUPS issue, but it doesn't address the SYS_SETUID and SYS_SETGID 16bit vs 32bit potential issue that I mentioned.

@bradfitz bradfitz reopened this Nov 9, 2016
@bradfitz bradfitz self-assigned this Nov 9, 2016
@gopherbot
Copy link

CL https://golang.org/cl/33011 mentions this issue.

@golang golang locked and limited conversation to collaborators Nov 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants