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

runtime/internal/atomic: on Linux/ARM, support kernel without CONFIG_KUSER_HELPERS? #23792

Closed
cherrymui opened this issue Feb 12, 2018 · 28 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@cherrymui
Copy link
Member

On Linux/ARM, we use the atomic operations provided by the kernel. (https://github.com/torvalds/linux/blob/master/arch/arm/kernel/entry-armv.S#L941)
This can be configured with CONFIG_KUSER_HELPERS in the kernel. On some machine, this option may not be enabled.

Should we support machines without this option enabled? Maybe only on GOARM=7, where the machine has atomic instructions? And keep relying on kernel helpers on older machines?

See also issue #23787.

cc @rsc @minux @davecheney

@cherrymui cherrymui added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 12, 2018
@cherrymui cherrymui added this to the Go1.11 milestone Feb 12, 2018
@aclements
Copy link
Member

The documentation for KUSER_HELPERS says "Warning: disabling this option may break user programs." Reading between the lines a bit, it seems like this config option should only be disabled in tightly integrated embedded situations where you control the software from top to bottom and know that nothing uses the helpers. It doesn't seem like it should be disabled for a general-purpose kernel. Given this, I don't think Go should be working around this config option.

@cherrymui
Copy link
Member Author

cherrymui commented Feb 12, 2018

Thanks, @aclements. I agree. Closing.

We can reopen if there is any problem.

@Risca
Copy link

Risca commented Feb 13, 2018

@aclements, @cherrymui, the documentation for CONFIG_KUSER_HELPERS also state:

However, the fixed address nature of these helpers can be used by ROP (return orientated programming) authors when creating exploits.

Taken from here: https://cateee.net/lkddb/web-lkddb/KUSER_HELPERS.html
I believe that's the reason our security policy is to disable it. Does that mean that our security policy is in conflict with running go binaries? Is there any workaround?

@frepe360
Copy link

CONFIG_KUSER_HELPERS is controlled by GRKERNSEC_OLD_ARM_USERLAND, which I have explicitly disable precisely because of the exploit possibility that @Risca mentions.

I have found that the protobuffer project had a similar issue with dependency on CONFIG_KUSER_HELPERS; and that redhat issued a patch to remove that dependency: https://gist.github.com/BennettSmith/7111094

Perhaps you would reconsider your decision to close this issue? I am very interested in running golang programs with a kernel that has been secured by disabling CONFIG_KUSER_HELPERS.

@Risca
Copy link

Risca commented Feb 13, 2018

Related pull request for protobuf: protocolbuffers/protobuf#2301

@aclements
Copy link
Member

GRKERNSEC_OLD_ARM_USERLAND is also documented as causing problems: "It's recommended that you try without this option enabled first, and only enable it if your userland does not boot (it will likely fail at init time)."

Has the C toolchain (gcc/clang and libc in particular) moved away from using these helpers?

There are good technical reasons for using these helpers: because of the myriad subtleties around the support for, performance of, and lack of forward-compatibility of ARM memory barriers, the kernel is in the best position to figure out what the right barriers are. We aren't simply using them out of convenience. But, if the C toolchain has figured out a good way to handle the barriers in user space, we could see what it's doing.

@frepe360
Copy link

@aclements: I see, thanks for this information. I do not know if the toolchain uses the helpers or not.

@Risca
Copy link

Risca commented Feb 13, 2018

@aclements, you cannot have KUSER_HELPERS enabled without also enabling GRKERNSEC_OLD_ARM_USERLAND. GRKERNSEC_OLD_ARM_USERLAND is listed as a dependency to KUSER_HELPERS in our kernel.

Symbol: KUSER_HELPERS [=n]
...
Depends on: MMU [=y] && (!CPU_V6 [=n] && !CPU_V6K [=n] && !CPU_V7 [=y] || GRKERNSEC_OLD_ARM_USERLAND [=n])

So, in order for us to use grsecurity and golang, we would need to enable GRKERNSEC_OLD_ARM_USERLAND which, as you pointed out, is not really recommended. I feel like we're stuck between a rock and a hard place.

I don't know the state of C toolchains regarding this. I wish I had the time to keep up.

I fully understand that there is a really valid use case for the kernel user helpers. Unfortunately for us, it also creates an incompatibility.
Could this possibly be made into a compiler option?

@cherrymui
Copy link
Member Author

cherrymui commented Feb 13, 2018

Could this possibly be made into a compiler option?

It's all in assembly, so no compiler option. A build tag might be more appropriate, if we end up with it (but I hope not).

If we go this way, would it be enough if we only do this on ARMv7? As @aclements mentioned, a good reason to use the kernel helper is not to dealing with the complexity of various incompatible memory barriers. If we do it on ARMv7 only, I think it is more feasible.

@frepe360
Copy link

I am mainly interested in ARM7.

@ianlancetaylor
Copy link
Contributor

For what it's worth, I checked what GCC does. It looks like if GCC is compiled with -march=armv6 or higher, it inlines the compare-and-exchange. With a lower -march value, it calls the kernel entry point unconditionally. This is based on compiling

_Bool f(int *p, int old, int new) {
  return __atomic_compare_exchange_n(p, &old, new, 0, __ATOMIC_ACQ_REL, __ATOMIC_RELAXED);
}

In principle we could check at run time which type of processor we were on, and select the compare-and-exchange implementation accordingly. But of course we would prefer to avoid that because it is more complex.

I don't think we should use build tags.

I don't see any good solution here. At least not until we can assume a sufficiently new ARM processor.

@frepe360
Copy link

Let me change that a bit; I am only interested in ARM7. So if there is a resonable way to do this for ARM7, I would be very happy if you would consider it.

@cherrymui
Copy link
Member Author

I think we can do this for ARMv7, by just testing runtime.goarm. It shouldn't be too complicated. I'll try making a CL.

@gopherbot
Copy link

Change https://golang.org/cl/94076 mentions this issue: runtime/internal/atomic: use native CAS on ARMv7

@paulzhol
Copy link
Member

+1 for issuing native instructions.
https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt states:

User space is expected to bypass those helpers and implement those things
inline (either in the code emitted directly by the compiler, or part of
the implementation of a library call) when optimizing for a recent enough
processor that has the necessary native support, but only if resulting
binaries are already to be incompatible with earlier ARM processors due to
usage of similar native instructions for other things.

This seems to be in line with the behavior in GCC @ianlancetaylor described.
Chromium also removed dependence on KUSER_HELPERS https://codereview.chromium.org/1840203004, relying on GCC intrinsics instead.

@ebati
Copy link

ebati commented Feb 15, 2018

musl libc can also be investigated. (here, here and here are some pointers to related codes. )

@aclements aclements reopened this Feb 15, 2018
@cherrymui
Copy link
Member Author

@frepe360, or anyone interested, could you try CL https://golang.org/cl/94076 (which depends on CL https://golang.org/cl/93637), to make sure it works on ARMv7 machines without the kernel helpers? I don't have the environment to test. Thanks.

@frepe360
Copy link

@cherrymui, of course, no problem. But I don't have access to it for a few days.

Until then; I understand that this is patch, correct? I have no experience in building a golang compiler. Can you explain what I need to do in order to test these changes?

@cherrymui
Copy link
Member Author

cherrymui commented Feb 16, 2018

@frepe360 thank you!

The easiest way is probably:

$ git clone https://go.googlesource.com/go # check out Go source tree, if not already done
$ cd go

Find the "DOWNLOAD" button on the webpage https://golang.org/cl/94076, click and copy the first command (currently as below), this will check out the patch.

$ git fetch https://go.googlesource.com/go refs/changes/76/94076/2 && git checkout FETCH_HEAD

Set up the bootstrap Go, if not already done (assuming you have Go installed on the machine):

$ export GOROOT_BOOTSTRAP=$(go env GOROOT)

Then build and test:

$ cd src
$ ./all.bash

The last step will probably take a while (hopefully).

@frepe360
Copy link

@cherrymui Ok, that's not going to be a problem.

@frepe360
Copy link

@cherrymui, just to be perfectly clear; I build this compiler according to your instructions. Then I compile a helloworld.go program with my homebuilt compiler, and then I try to run the resulting helloworld program on an ARM7 with my specific kernel configuration.

Correct?

@cherrymui
Copy link
Member Author

@frepe360 the instruction above was to build natively on ARM machine. It will also work for cross-compilation. Once you build the compiler as above, just do

$ GOARCH=arm GOOS=linux GOARM=7 /path/to/new/go build helloworld.go

If you could try some programs more than a helloworld, that will be great. Thanks!

@frepe360
Copy link

Unfortunately, I get a build error here....

git clone https://go.googlesource.com/go
cd go
git fetch https://go.googlesource.com/go refs/changes/37/93637/4 && git checkout FETCH_HEAD
git fetch https://go.googlesource.com/go refs/changes/76/94076/2 && git checkout FETCH_HEAD
cd src
./all.bash

... gives me this:

Building Go cmd/dist using /usr/lib/go-1.6.
Building Go toolchain1 using /usr/lib/go-1.6.

Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/arm.

Testing packages.

ok archive/tar 0.786s
ok archive/zip 35.433s
ok bufio 3.822s
ok bytes 9.887s
ok compress/bzip2 0.827s
ok compress/flate 8.000s
ok compress/gzip 0.187s
ok compress/lzw 0.063s
ok compress/zlib 0.204s
ok container/heap 0.090s
ok container/list 0.060s
ok container/ring 0.133s
ok context 1.027s
ok crypto 0.053s
ok crypto/aes 0.116s
ok crypto/cipher 0.105s
ok crypto/des 0.138s
ok crypto/dsa 0.104s
ok crypto/ecdsa 2.437s
ok crypto/elliptic 0.386s
ok crypto/hmac 0.028s
ok crypto/md5 0.039s
ok crypto/rand 0.627s
ok crypto/rc4 0.849s
ok crypto/rsa 1.609s
ok crypto/sha1 0.063s
ok crypto/sha256 0.064s
ok crypto/sha512 0.073s
ok crypto/subtle 0.050s
ok crypto/tls 13.494s
ok crypto/x509 21.999s
ok database/sql 1.493s
ok database/sql/driver 0.054s
ok debug/dwarf 0.129s
ok debug/elf 0.177s
ok debug/gosym 0.090s
ok debug/macho 0.049s
ok debug/pe 0.074s
ok debug/plan9obj 0.025s
ok encoding/ascii85 0.068s
ok encoding/asn1 0.086s
ok encoding/base32 0.046s
ok encoding/base64 0.150s
ok encoding/binary 0.055s
ok encoding/csv 0.082s
ok encoding/gob 0.490s
ok encoding/hex 0.125s
ok encoding/json 4.820s
ok encoding/pem 0.152s
ok encoding/xml 0.380s
ok errors 0.041s
ok expvar 0.065s
ok flag 0.836s
ok fmt 0.892s
ok go/ast 0.063s
ok go/build 2.219s
ok go/constant 0.036s
ok go/doc 0.859s
ok go/format 0.087s
ok go/importer 0.826s
ok go/internal/gccgoimporter 0.150s
ok go/internal/gcimporter 3.134s
ok go/internal/srcimporter 6.119s
ok go/parser 0.346s
ok go/printer 3.166s
ok go/scanner 0.078s
ok go/token 0.231s
ok go/types 9.519s
ok hash 0.030s
ok hash/adler32 1.196s
ok hash/crc32 0.056s
ok hash/crc64 0.036s
ok hash/fnv 0.022s
ok html 0.027s
ok html/template 0.426s
ok image 1.251s
ok image/color 0.222s
ok image/draw 0.564s
ok image/gif 5.666s
ok image/jpeg 2.160s
ok image/png 0.388s
ok index/suffixarray 0.095s
ok internal/cpu 0.037s
ok internal/poll 0.168s
ok internal/singleflight 0.082s
ok internal/trace 18.736s
ok io 0.123s
ok io/ioutil 0.065s
ok log 0.084s
ok log/syslog 1.407s
ok math 0.057s
ok math/big 25.209s
ok math/bits 0.103s
ok math/cmplx 0.039s
ok math/rand 2.350s
ok mime 0.109s
ok mime/multipart 5.509s
ok mime/quotedprintable 1.612s
ok net 5.359s
ok net/http 22.371s
ok net/http/cgi 3.747s
ok net/http/cookiejar 0.087s
ok net/http/fcgi 0.082s
ok net/http/httptest 0.420s
ok net/http/httptrace 0.038s
ok net/http/httputil 0.539s
ok net/http/internal 0.035s
ok net/internal/socktest 0.028s
ok net/mail 0.092s
ok net/rpc 0.219s
ok net/rpc/jsonrpc 0.115s
ok net/smtp 0.313s
ok net/textproto 0.072s
ok net/url 0.150s
ok os 1.399s
ok os/exec 2.843s
ok os/signal 6.875s
ok os/user 0.044s
ok path 0.035s
ok path/filepath 0.245s
ok reflect 1.489s
ok regexp 1.782s
ok regexp/syntax 6.697s
ok runtime 138.645s
ok runtime/debug 0.085s
ok runtime/internal/atomic 0.774s
ok runtime/internal/sys 0.058s
ok runtime/pprof 16.114s
ok runtime/pprof/internal/profile 0.063s
ok runtime/trace 26.836s
ok sort 1.100s
ok strconv 4.799s
ok strings 2.449s
ok sync 1.576s
ok sync/atomic 0.673s
ok syscall 0.251s
ok testing 1.506s
ok testing/quick 1.433s
ok text/scanner 0.042s
ok text/tabwriter 0.084s
ok text/template 4.087s
ok text/template/parse 0.097s
ok time 8.371s
ok unicode 0.134s
ok unicode/utf16 0.092s
ok unicode/utf8 0.149s
ok vendor/golang_org/x/crypto/chacha20poly1305 2.369s
ok vendor/golang_org/x/crypto/chacha20poly1305/internal/chacha20 0.018s
ok vendor/golang_org/x/crypto/cryptobyte 0.097s
ok vendor/golang_org/x/crypto/curve25519 1.885s
ok vendor/golang_org/x/crypto/poly1305 0.024s
ok vendor/golang_org/x/net/http2/hpack 0.049s
ok vendor/golang_org/x/net/idna 0.040s
ok vendor/golang_org/x/net/lex/httplex 0.034s
ok vendor/golang_org/x/net/nettest 2.943s
ok vendor/golang_org/x/net/proxy 0.044s
ok vendor/golang_org/x/text/transform 0.046s
ok vendor/golang_org/x/text/unicode/norm 0.024s
ok cmd/addr2line 13.561s
ok cmd/api 0.099s
ok cmd/asm/internal/asm 4.143s
ok cmd/asm/internal/lex 0.035s
go build cmd/compile/internal/gc: /home/ubuntu/src/go/pkg/tool/linux_arm/compile: signal: killed
go build cmd/compile/internal/ssa: /home/ubuntu/src/go/pkg/tool/linux_arm/compile: signal: killed
ok cmd/compile 344.411s
FAIL cmd/compile/internal/gc [build failed]
FAIL cmd/compile/internal/ssa [build failed]
ok cmd/compile/internal/syntax 0.101s
ok cmd/compile/internal/test 0.052s [no tests to run]
ok cmd/compile/internal/types 0.043s
ok cmd/cover 50.191s
ok cmd/doc 1.463s
ok cmd/fix 261.314s
ok cmd/go 30.287s
ok cmd/go/internal/cache 4.047s
ok cmd/go/internal/generate 0.066s
ok cmd/go/internal/get 0.635s
ok cmd/go/internal/load 0.046s
ok cmd/go/internal/work 0.174s
ok cmd/gofmt 0.482s
ok cmd/internal/buildid 4.004s
ok cmd/internal/dwarf 0.028s
ok cmd/internal/edit 0.027s
ok cmd/internal/goobj 5.426s
ok cmd/internal/obj 0.026s
ok cmd/internal/obj/arm64 0.053s
ok cmd/internal/obj/x86 20.844s
ok cmd/internal/objabi 0.021s
ok cmd/internal/src 0.022s
ok cmd/internal/test2json 1.697s
ok cmd/link 15.828s
ok cmd/link/internal/ld 166.859s
ok cmd/nm 28.347s
ok cmd/objdump 16.980s
ok cmd/pack 15.810s
ok cmd/trace 0.121s
ok cmd/vendor/github.com/google/pprof/internal/binutils 0.109s
ok cmd/vendor/github.com/google/pprof/internal/driver 13.971s
ok cmd/vendor/github.com/google/pprof/internal/elfexec 0.031s
ok cmd/vendor/github.com/google/pprof/internal/graph 0.688s
ok cmd/vendor/github.com/google/pprof/internal/measurement 0.035s
ok cmd/vendor/github.com/google/pprof/internal/report 0.129s
ok cmd/vendor/github.com/google/pprof/internal/symbolizer 0.096s
ok cmd/vendor/github.com/google/pprof/internal/symbolz 0.110s
ok cmd/vendor/github.com/google/pprof/profile 0.935s
ok cmd/vendor/github.com/ianlancetaylor/demangle 0.194s
ok cmd/vendor/golang.org/x/arch/arm/armasm 0.214s
ok cmd/vendor/golang.org/x/arch/arm64/arm64asm 1.728s
ok cmd/vendor/golang.org/x/arch/ppc64/ppc64asm 0.064s
ok cmd/vendor/golang.org/x/arch/x86/x86asm 1.892s
ok cmd/vet 21.210s
ok cmd/vet/internal/cfg 0.080s
2018/02/16 20:36:23 Failed: exit status 1

@frepe360
Copy link

(This was done on an ARM7, a raspberry pi. It does not have my "special" kernel.)

@cherrymui
Copy link
Member Author

go build cmd/compile/internal/gc: /home/ubuntu/src/go/pkg/tool/linux_arm/compile: signal: killed
go build cmd/compile/internal/ssa: /home/ubuntu/src/go/pkg/tool/linux_arm/compile: signal: killed

I guess they are OOM'd, or timeout (which is possible on raspberry pi). Anyway, these are just tests, now you should already have the compiler built. You can use this new Go toolchain to build helloworld and run on the device with the special kernel. Thanks!

@frepe360
Copy link

Ok, no problem. I'll let you know on monday.

@frepe360
Copy link

The helloworld built with the compiler that I build according to your instructions, does NOT segfault on my kernel. The helloworld built with the regular compiler, DOES segfault:

/tmp/nfs # ./helloworld-arm7
Segmentation fault
/tmp/nfs # ./helloworld-ARM7-new-go-compiler 
hello world
/tmp/nfs # 

I assume that this is progress?

@cherrymui
Copy link
Member Author

Thanks, @frepe360! This is great.

@golang golang locked and limited conversation to collaborators May 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

8 participants