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: default buildmode incompatible to external linker with PIE enabled by default #17847

Closed
williamweixiao opened this issue Nov 8, 2016 · 21 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@williamweixiao
Copy link
Member

Please answer these questions before submitting your issue. Thanks!

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

go version go1.7.3 linux/arm64

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

GOARCH="arm64"
GOBIN=""
GOEXE=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GORACE=""
GOROOT="/root/go-go1.7.3"
GOTOOLDIR="/root/go-go1.7.3/pkg/tool/linux_arm64"
CC="gcc"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build719384124=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

Following 3 steps

  1. Need a GCC with PIE enabled by default
    e.g.,
    Using built-in specs.
    COLLECT_GCC=gcc
    COLLECT_LTO_WRAPPER=/usr/libexec/gcc/aarch64-alpine-linux-musl/6.2.1/lto-wrapper
    Target: aarch64-alpine-linux-musl
    Configured with: /home/buildozer/aports/main/gcc/src/gcc-6.2.0/configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --build=aarch64-alpine-linux-musl --host=aarch64-alpine-linux-musl --target=aarch64-alpine-linux-musl --with-pkgversion='Alpine 6.2.1' --enable-checking=release --disable-fixed-point --disable-libstdcxx-pch --disable-multilib --disable-nls --disable-werror --disable-symvers --enable-__cxa_atexit --enable-default-pie --enable-cloog-backend --enable-languages=c,c++,objc,java,fortran,ada --with-arch=armv8-a --with-abi=lp64 --disable-bootstrap --disable-libquadmath --disable-libssp --disable-libmpx --disable-libmudflap --disable-libsanitizer --enable-shared --enable-threads --enable-tls --with-system-zlib --with-linker-hash-style=gnu
    Thread model: posix
    gcc version 6.2.1 20160822 (Alpine 6.2.1)

  2. Build Go from source code (./make.bash)

  3. run go command (./go)

What did you expect to see?

run successfully

What did you see instead?

Segmentation fault

@williamweixiao
Copy link
Member Author

The issue is discussed at: https://groups.google.com/forum/#!topic/golang-dev/gRCe5URKewI

@williamweixiao williamweixiao changed the title Default buildmode incompatible with external linker with PIE enabled by default Default buildmode incompatible to external linker with PIE enabled by default Nov 8, 2016
@ianlancetaylor ianlancetaylor changed the title Default buildmode incompatible to external linker with PIE enabled by default cmd/go: default buildmode incompatible to external linker with PIE enabled by default Nov 8, 2016
@ianlancetaylor ianlancetaylor added this to the Go1.8Maybe milestone Nov 8, 2016
@ianlancetaylor
Copy link
Contributor

Looking further into this, I realize that there are still things I don't understand.

By default, the go tool is not built in external linking mode. Why is that happening for you?

When I run go build -ldflags=-v cmd/go I can see from the linker output that it is using internal linking mode, meaning that the external linker is never invoked, meaning that the default behavior of GCC is irrelevant.

By default, Go code is compiled such that it can be successfully linked into a PIE. Why is that not happening for you?

When I run go build -ldflags="-linkmode=external -extldflags=-pie -v" cmd/go I can see that the external linker is invoked, that -pie is passed to it, and that the result is a PIE. It runs correctly--it does not get a segmentation fault.

Why do your go tool crash? Can you run it under gdb and see where it goes wrong?

@quentinmit quentinmit added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 8, 2016
@mwhudson
Copy link
Contributor

mwhudson commented Nov 8, 2016

Something that's not made obvious enough in the report is that this is on aarch64 on alpine, so with musl as libc. But even so I don't know why this wouldn't work, Ian's points are still very relevant.

@williamweixiao
Copy link
Member Author

williamweixiao commented Nov 9, 2016

There are three preconditions to reproduce the crash

  1. ARM64 or MIPS64 platforms on which go will invoke external linker
  2. Test program should use CGO (such as "go" itself) which depend on external linker
  3. GCC toolchain enable PIE by default (which is the case for Alpine and other security hardened Linux distros)

I have used GDB to analyze the crash and find that it's due to following abnormal ELF relocation items:

Relocation section '.rela.dyn' at offset 0xf98 contains 67861 entries:
Offset Info Type Sym. Value Sym. Name + Addend
000000539108 000000000403 R_AARCH64_RELATIV 1fc090
000000539110 000000000403 R_AARCH64_RELATIV 1faae0

......
These relocation items are write-protections according to program headers as below:

Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000
0x00000000007a7708 0x00000000007a7708 R E 10000

So the crash occurs when musl ld is trying to fill relocation address to these items at runtime
I can fix the crash if i rungo build -ldflags="-extldflags=-no-pie" cmd/go

Alpine community also has a workaround for the crash, as can be seen here: http://git.alpinelinux.org/cgit/aports/tree/community/go/default-buildmode-pie.patch
But i think the workaround can't be acceptable by other platforms since it changes default buildmode

It's interesting that X86_64 linker seems to be able to handle the "PIE" linking for "exe" buildmode. But l'd say Go needs to explicitly disable "PIE" for "exe" buildmode instead of relying on the default behavior of external linker.

So i suggest fixing the issue by turning PIE off explicitly via "-no-pie" (if recognized by GCC) when invoking external linker for "exe" buildmode.

@ianlancetaylor
Copy link
Contributor

A similar thing happens on amd64--there are RELATIVE relocs against the text segment. It works nonetheless because the program has a DT_TEXTREL entry in the dynamic segment.

Can you run readelf -d on your executable to see if TEXTREL is there?

Do you know if there is something on your system that disables DT_TEXTREL processing in the dynamic linker on your system?

@ianlancetaylor
Copy link
Contributor

@mwhudson @crawshaw I wonder if we should just make UseRelro always return true on an ELF system when using external linking.

@mwhudson
Copy link
Contributor

mwhudson commented Nov 9, 2016

Sounds like a good idea to me, I think (this stuff is fairly confusing!).

Does the new linkmode=internal buildmode=pie do the relro stuff?

@ianlancetaylor
Copy link
Contributor

Yes, UseRelro returns true if Buildmode == BuildmodePIE.

@gopherbot
Copy link
Contributor

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

@ianlancetaylor
Copy link
Contributor

The relro idea doesn't work, because on some platforms you have to compile with -shared in order to get an PIE that does not have a DT_TEXTREL dynamic tag.

I'd still like to understand exactly what is going wrong on Alpine GNU/Linux.

@mwhudson
Copy link
Contributor

mwhudson commented Nov 9, 2016

On 10 November 2016 at 11:27, Ian Lance Taylor notifications@github.com
wrote:

The relro idea doesn't work, because on some platforms you have to compile
with -shared in order to get an PIE that does not have a DT_TEXTREL
dynamic tag.

I'd still like to understand exactly what is going wrong on Alpine
GNU/Linux.

I googled about a bit and it seems that perhaps PIE executables with
TEXTREL just don't work there (something to do with the kernel patches they
use?): https://github.com/dotnet/coreclr/issues/6211#issuecomment-231618515

(BTW, because I'd forgotten this temporarily, the reason this shows up more
on arm64 is because there is no support for internal linking with cgo at
all there).

@ianlancetaylor
Copy link
Contributor

Well, but DT_TEXTREL is implemented entirely in the dynamic linker. It's not a kernel feature at all. Of course they could have a patch to their dynamic linker.

@mwhudson
Copy link
Contributor

I think it's more that mprotect(PROT_WRITE|PROT_EXEC) is ignored or
rejected or something like that.

On 10 November 2016 at 12:20, Ian Lance Taylor notifications@github.com
wrote:

Well, but DT_TEXTREL is implemented entirely in the dynamic linker. It's
not a kernel feature at all. Of course they could have a patch to their
dynamic linker.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17847 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AApBFmTBLUDT_JCIOQGwbjfiJMwaE373ks5q8lVGgaJpZM4KsLrU
.

@ianlancetaylor
Copy link
Contributor

The dynamic linker doesn't do that for DT_TEXTREL. It calls mprotect(PROT_READ|PROT_WRITE), applies the relocations, then calls mprotect(PROT_READ|PROT_EXEC).

@mwhudson
Copy link
Contributor

Ah, but musl's does not do that: https://git.musl-libc.org/cgit/musl/tree/ldso/dynlink.c#n699 (perhaps someone should tell them?)

@williamweixiao
Copy link
Member Author

@ianlancetaylor

# readelf -d ./go

Dynamic section at offset 0x7a7cb8 contains 25 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libc.so]
 0x000000000000000c (INIT)               0x18ec60
 0x000000000000000d (FINI)               0x537f28
 0x0000000000000019 (INIT_ARRAY)         0x7b7ca0
 0x000000000000001b (INIT_ARRAYSZ)       8 (bytes)
 0x000000000000001a (FINI_ARRAY)         0x7b7ca8
 0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
 0x000000006ffffef5 (GNU_HASH)           0x298
 0x0000000000000005 (STRTAB)             0xb30
 0x0000000000000006 (SYMTAB)             0x3f8
 0x000000000000000a (STRSZ)              1122 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000015 (DEBUG)              0x0
 0x0000000000000003 (PLTGOT)             0x7b7e88
 0x0000000000000002 (PLTRELSZ)           720 (bytes)
 0x0000000000000014 (PLTREL)             RELA
 0x0000000000000017 (JMPREL)             0x18e990
 0x0000000000000007 (RELA)               0xf98
 0x0000000000000008 (RELASZ)             1628664 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x0000000000000016 (TEXTREL)            0x0
 0x0000000000000018 (BIND_NOW)
 0x000000006ffffffb (FLAGS_1)            Flags: NOW PIE
 0x000000006ffffff9 (RELACOUNT)          67835
 0x0000000000000000 (NULL)               0x0

@williamweixiao
Copy link
Member Author

williamweixiao commented Nov 10, 2016

@mwhudson @ianlancetaylor
Alpine musl ld do mprotect(map, map_len, PROT_READ|PROT_WRITE|PROT_EXEC) for DT_TEXTREL at: https://git.musl-libc.org/cgit/musl/tree/ldso/dynlink.c#n699
But with gdb to trace the execution, i find the code segment has no chance to execute since the main program was already loaded by the kernel https://git.musl-libc.org/cgit/musl/tree/ldso/dynlink.c#n1433

GoLang has provided a special GOOS option for Android platform to switch from "exe" to "pie". Is the switch for avoiding the similar situation as Alpine of unsupporting TEXTREL?

Btw, some platforms may forbid TEXTREL on purpose since it is wasting memory and prevents security hardening.

@ianlancetaylor
Copy link
Contributor

I don't understand what you mean by "the code segment has no chance to execute."

Can you confirm that Alpine GNU/Linux rejects an mprotect call that tries to set both PROT_EXEC and PROT_WRITE?

If that is so, then it seems that the core problem here is that the musl dynamic linker has a bug in its implementation of DT_TEXTREL. You suggest that some platforms may reject DT_TEXTREL, but I would prefer to not take any action until we encounter such platforms.

Setting GOOS=android defaults to PIE because Android requires PIE. See #10807. There are various other differences with android, which is not a standard GNU/Linux platform. I would very much prefer to avoid adding GOOS=alpine.

In any case, does https://golang.org/cl/33106 work around the problem enough to let Go programs run?

@gopherbot
Copy link
Contributor

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

@williamweixiao
Copy link
Member Author

"go" is firstly loaded by Alpine kernel with "R E" attribute to its code segment and then execution flow is passed to musl linker. There is no mprotect call trying to set both PROT_EXEC and PRO_WRITE inside musl linker before doing relocation. We are trying to contact Alpine guys to understand what's the reason of unsupporting DT_TEXTREL (a bug or a designed feature)

Anyway, your cl (https://golang.org/cl/33106) works around the problem and Go programs run in my environment (Alpine ARM64) now

weixiaowilliam pushed a commit to weixiaowilliam/golang that referenced this issue Dec 1, 2016
For two reasons:
1) Alpine has fixed the issue (http://git.alpinelinux.org/cgit/aports/tree/community/go/default-buildmode-pie.patch)
2) The patch doesn't work for some platforms where Golang adopts external linker (golang/go#17847). Although Golang developers has fixed the issue with disabling PIE, above Alpine solution is more secure

So, Alpine Go (installed by apk) is better choice.
weixiaowilliam pushed a commit to weixiaowilliam/golang that referenced this issue Dec 1, 2016
For two reasons:
1) Alpine has fixed the issue (http://git.alpinelinux.org/cgit/aports/tree/community/go/default-buildmode-pie.patch)
2) The patch doesn't work for some platforms where Golang adopts external linker (golang/go#17847). Although Golang developers has fixed the issue with disabling PIE, above Alpine solution is more secure

So, Alpine Go (installed by apk) is better choice.
@justincormack
Copy link

cc @ncopa

@golang golang locked and limited conversation to collaborators Jan 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants