syscall: on linux 386 doesn't support syscalls that don't fail #22924

chipaca · 2017-11-29T16:59:45Z

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

1.6.2 and 1.9.2

Does this issue reproduce with the latest release?

yes

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

linux/386 and linux/arm.

What did you do?

with this built as hightest,

package main

import (
	"fmt"
	"syscall"
)

func main() {
	r0, _, e := syscall.RawSyscall(syscall.SYS_GETUID32, 0, 0, 0)
	fmt.Println(uint32(r0), "/", e)
}

and a user with a high uid,

sudo useradd --uid "$(( 0xfffffffe ))" --shell /bin/sh hightest

(alternatively if you can't useradd, use chown to a numerical id, and chmod u+s, and change the code to use GETEUID instead of GETUID)

compare the output of sudo -u hightest id -u and sudo -u hightest hightest.

What did you expect to see?

4294967294 / errno 0

What did you see instead?

4294967295 / no such file or directory

The problem seems to be that in syscall/asm_linux_386.s, if the returned value is not less than 0xfffff001 it's treated as an error.

The text was updated successfully, but these errors were encountered:

bradfitz · 2017-11-29T19:10:01Z

/cc @ianlancetaylor

ianlancetaylor · 2017-11-29T19:35:47Z

The same thing happens on amd64 if there are any syscalls that can return a valid value larger than 0xfffffffffffff001.

ianlancetaylor · 2017-11-29T19:51:58Z

OK, I see. The code in the syscall package is correct for syscalls that can return an error. However, there are certain syscalls that do not return an error. For those, we should not check the result in this way. Those syscalls are:

clock_gettime (amd64 only, I think)
getegid
getegid32 (386 only)
geteuid
geteuid32 (386 only)
getgid
getgid32 (386 only)
getpgrp
getpid
getppid
getuid
getuid32 (386 only)
personality (386 only, I think)
setfsgid32 (386 only)
setfsuid32 (386 only)
umask

ianlancetaylor · 2017-11-29T19:55:57Z

Clearly we should fix Getuid, et. al., in the syscall package. It's less clear whether we should add a new API function such as SyscallNoError to the syscall package, or reserve that for x/sys/unix.

bradfitz · 2017-11-29T19:59:36Z

x/sys/unix for sure.

We can add [more] comments to syscall pointing to x/sys/unix where needed.

gopherbot · 2017-12-18T11:07:58Z

Change https://golang.org/cl/84485 mentions this issue: syscall: support syscalls without error return on Linux

tklauser · 2017-12-18T11:13:00Z

I submitted https://golang.org/cl/84485 with a possible fix in syscall, marked for go 1.11 (but not ready for review yet). I'd like to add a test, but I couldn't yet figure out a proper way to add one based on the reproducer provided in this issue. Will hopefully find some time to work on that during the holiday season.

tklauser · 2017-12-19T16:33:29Z

Tested on linux/386 (via gomote) with https://golang.org/cl/84485 and the following program:

package main

import (
	"fmt"
	"syscall"
)

func main() {
	// r0, _, e := syscall.RawSyscall(syscall.SYS_GETUID32, 0, 0, 0)
	// fmt.Println(uint32(r0), "/", e)
	u := syscall.Getuid()
	fmt.Println(uint32(u))
}

and indeed it print 4294967294 as expected.

If anyone has an idea on how to turn the provided reproducer into a nice test, any hints would be appreciated.

I'll also work on a corresponding implementation for x/sys/unix, adding an exported RawSyscallNoError so the following program should also print 4294967294 / 0

package main

import (
	"fmt"
	"golang.org/x/sys/unix"
)

func main() {
	r0, _, e := unix.RawSyscallNoError(syscall.SYS_GETUID32, 0, 0, 0)
	fmt.Println(uint32(r0), "/", e)
}

chipaca · 2017-12-19T17:16:34Z

How is the syscall package tested? This isn't the only syscall that'd need special privileges to test the corner cases, I'm sure.

tklauser · 2017-12-19T18:52:24Z

The syscall is tested via the tests in syscall/*_test.go and some functionality is also implicitly tested via the os package tests. There are also some tests regarding syscall in test/ (e.g. test/fixedbugs/issue15002.go). But all of these run as a non-privileged user AFAIK.

As for the syscall to use (from @ianlancetaylor's list in #22924 (comment)), I think only the gete?[gu]id, the personality (which seems to be available on all GOARCHs btw) and the umask syscalls let userspace influence what value the kernel will return. And umaskwill probably not even allow us to set a high enough value to trigger the issue.

bradfitz · 2017-12-19T19:17:17Z

@chipaca, historically the syscall package has had few explicit tests. Most tests had been implicit ones, tested via upper layers working correctly. That's not ideal and has slowly changed, but there's still not a lot of explicit testing. It also doesn't change often, so manual testing has so far sufficed. (Again, not ideal.)

@tklauser, some of of builders do run as root, but most of our Linux ones run as a sort of "fake root" in Kubernetes that is UID 0 but with a lot fewer capabilities enabled. We could re-enable a Linux builder in a VM running as root if needed, and then we can test for being in that VM via testenv.Builder().

gopherbot · 2018-01-02T11:13:41Z

Change https://golang.org/cl/85756 mentions this issue: unix: add SyscallNoError and RawSyscallNoError on Linux

Add the SyscallNoError and RawSyscallNoError wrapper functions which are used for syscalls that don't return an error. Also convert all applicable syscalls to use these wrappers. Updates golang/go#22924 Change-Id: Ie143abc99de8d12d36b67ac278e08d7565f73ce0 Reviewed-on: https://go-review.googlesource.com/85756 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

gopherbot · 2018-01-29T10:14:27Z

Change https://golang.org/cl/90475 mentions this issue: unix: use SyscallNoError and RawSyscallNoError on Linux only

The SyscallNoError and RawSyscallNoError functions are only needed on Linux. Make sure they are not used within the generated zsyscall_*.go files if $GOOS != "linux". Updates golang/go#22924 Change-Id: I3b2aa8624895b0527dcc832b2945a8d591f0ba1a Reviewed-on: https://go-review.googlesource.com/90475 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Add the rawSyscallNoError wrapper function which is used for Linux syscalls that don't return an error and convert all applicable occurences of RawSyscall to use it instead. Fixes golang#22924 Change-Id: Iff1eddb54573d459faa01471f10398b3d38528dd

tklauser · 2018-01-30T16:03:21Z

Patchset 4 of https://golang.org/cl/84485 now implements the alternative geteuid test as suggested by @chipaca. Currently it only checks whether it is run as root via if os.Getuid() != 0 but with a TODO(tklauser) to eventually change it as @bradfitz suggested, in case we need "non-fake" root. Let's see what the trybots think...

tklauser · 2018-01-30T16:15:03Z

The trybots look happy but I'm not sure how to check whether they actually ran the test in question (TestSyscallNoError). @bradfitz, is there a way to check?

FWIW, the test passes locally when run as root on linux/arm or linux/386.

bradfitz · 2018-01-30T16:39:12Z

@tklauser, I replied at https://go-review.googlesource.com/c/go/+/84485#message-586556e53a5403fa6b211cfccfcaef20ecdc8dc5

gopherbot · 2018-06-13T13:46:42Z

Change https://golang.org/cl/118655 mentions this issue: syscall: support Linux syscalls without error return on mipsx/mips64x

Like on other architectures, use rawSyscallNoError for Linux syscalls that don't return an error and convert all applicable occurences of RawSyscall to use it instead. This was missed in CL 84485 because mkall.sh doesn't support mipsx/mips64x, so add the corresponding entries as well. Updates #22924 Change-Id: I762cbee0827140b9890c4a10830e0b4cd33de92f Reviewed-on: https://go-review.googlesource.com/118655 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Like on other architectures, use rawSyscallNoError for Linux syscalls that don't return an error and convert all applicable occurences of RawSyscall to use it instead. This was missed in CL 84485 because mkall.sh doesn't support mipsx/mips64x, so add the corresponding entries as well. Updates golang#22924 Change-Id: I762cbee0827140b9890c4a10830e0b4cd33de92f Reviewed-on: https://go-review.googlesource.com/118655 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Go 1.11 added the rawSyscallNoError wrapper function which is used for Linux syscalls that don't return an error and converted all applicable occurences of RawSyscall to use it instead. This change implements it for GopherJS on Linux. Also implement unsetenv_c, analogous to setenv_c, so that os.Unsetenv can be supported with Node.js, just like os.Setenv already was. Fixes: $ GOOS=linux gopherjs test --short io Error: runtime error: native function not implemented: syscall.rawSyscallNoError FAIL io 1.036s $ goexec -compiler=gopherjs 'os.Unsetenv("TEST")' Error: runtime error: native function not implemented: syscall.unsetenv_c Follows golang/go@36951a9. Updates golang/go#22924.

chipaca mentioned this issue Nov 29, 2017

osutil/sys: reimplement getuid and chown with the right int type canonical/snapd#4291

Merged

bradfitz changed the title ~~syscall on linux 386 doesn't support syscalls that don't fail~~ syscall: on linux 386 doesn't support syscalls that don't fail Nov 29, 2017

bradfitz added this to the Go1.11 milestone Nov 29, 2017

bradfitz added the NeedsInvestigation label Nov 29, 2017

ianlancetaylor added release-blocker NeedsFix labels Nov 29, 2017

gopherbot removed the NeedsInvestigation label Nov 29, 2017

tklauser self-assigned this Dec 18, 2017

gopherbot closed this as completed in 36951a9 Feb 13, 2018

golang locked and limited conversation to collaborators Jun 13, 2019

gopherbot added the FrozenDueToAge label Jun 13, 2019

rsc unassigned tklauser Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syscall: on linux 386 doesn't support syscalls that don't fail #22924

syscall: on linux 386 doesn't support syscalls that don't fail #22924

chipaca commented Nov 29, 2017 •

edited

Loading

bradfitz commented Nov 29, 2017

ianlancetaylor commented Nov 29, 2017

ianlancetaylor commented Nov 29, 2017

ianlancetaylor commented Nov 29, 2017

bradfitz commented Nov 29, 2017

gopherbot commented Dec 18, 2017

tklauser commented Dec 18, 2017

tklauser commented Dec 19, 2017 •

edited

Loading

chipaca commented Dec 19, 2017

tklauser commented Dec 19, 2017 •

edited

Loading

bradfitz commented Dec 19, 2017

gopherbot commented Jan 2, 2018

gopherbot commented Jan 29, 2018

tklauser commented Jan 30, 2018

tklauser commented Jan 30, 2018

bradfitz commented Jan 30, 2018

gopherbot commented Jun 13, 2018

syscall: on linux 386 doesn't support syscalls that don't fail #22924

syscall: on linux 386 doesn't support syscalls that don't fail #22924

Comments

chipaca commented Nov 29, 2017 • edited Loading

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

bradfitz commented Nov 29, 2017

ianlancetaylor commented Nov 29, 2017

ianlancetaylor commented Nov 29, 2017

ianlancetaylor commented Nov 29, 2017

bradfitz commented Nov 29, 2017

gopherbot commented Dec 18, 2017

tklauser commented Dec 18, 2017

tklauser commented Dec 19, 2017 • edited Loading

chipaca commented Dec 19, 2017

tklauser commented Dec 19, 2017 • edited Loading

bradfitz commented Dec 19, 2017

gopherbot commented Jan 2, 2018

gopherbot commented Jan 29, 2018

tklauser commented Jan 30, 2018

tklauser commented Jan 30, 2018

bradfitz commented Jan 30, 2018

gopherbot commented Jun 13, 2018

chipaca commented Nov 29, 2017 •

edited

Loading

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

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

tklauser commented Dec 19, 2017 •

edited

Loading

tklauser commented Dec 19, 2017 •

edited

Loading