From 32fb19371bc2901910de79444e87ee84779ce24c Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 30 Sep 2014 12:43:49 -0400 Subject: [release-branch.go1.3] cmd/5l, cmd/6l, cmd/8l: fix nacl binary corruption bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ««« CL 135050043 / 57dfd03985a9 cmd/5l, cmd/6l, cmd/8l: fix nacl binary corruption bug NaCl requires the addition of a 32-byte "halt sled" at the end of the text segment. This means that segtext.len is actually 32 bytes shorter than reality. The computation of the file offset of the end of the data segment did not take this 32 bytes into account, so if len and len+32 rounded up (by 64k) to different values, the symbol table overwrote the last page of the data segment. The last page of the data segment is usually the C .string symbols, which contain the strings used in error prints by the runtime. So when this happens, your program probably crashes, and then when it does, you get binary garbage instead of all the usual prints. The chance of hitting this with a randomly sized text segment is 32 in 65536, or 1 in 2048. If you add or remove ANY code while trying to debug this problem, you're overwhelmingly likely to bump the text segment one way or the other and make the bug disappear. Correct all the computations to use segdata.fileoff+segdata.filelen instead of trying to rederive segdata.fileoff. This fixes the failure during the nacl/amd64p32 build. TBR=iant CC=golang-codereviews https://golang.org/cl/135050043 »»» LGTM=bradfitz R=golang-codereviews CC=adg, bradfitz, golang-codereviews, iant https://golang.org/cl/151150044 --- src/cmd/5l/asm.c | 4 ++-- src/cmd/6l/asm.c | 8 ++++---- src/cmd/8l/asm.c | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cmd/5l/asm.c b/src/cmd/5l/asm.c index 5e9267b5ba..e879212188 100644 --- a/src/cmd/5l/asm.c +++ b/src/cmd/5l/asm.c @@ -599,10 +599,10 @@ asmb(void) if(iself) goto ElfSym; case Hplan9: - symo = HEADR+segtext.len+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; break; ElfSym: - symo = rnd(HEADR+segtext.filelen, INITRND)+rnd(HEADR+segrodata.filelen, INITRND)+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; symo = rnd(symo, INITRND); break; } diff --git a/src/cmd/6l/asm.c b/src/cmd/6l/asm.c index e251e32ca9..7828e28922 100644 --- a/src/cmd/6l/asm.c +++ b/src/cmd/6l/asm.c @@ -689,10 +689,10 @@ asmb(void) case Hplan9: case Helf: debug['s'] = 1; - symo = HEADR+segtext.len+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; break; case Hdarwin: - symo = rnd(HEADR+segtext.len, INITRND)+rnd(segdata.filelen, INITRND)+machlink; + symo = segdata.fileoff+rnd(segdata.filelen, INITRND)+machlink; break; case Hlinux: case Hfreebsd: @@ -701,11 +701,11 @@ asmb(void) case Hdragonfly: case Hsolaris: case Hnacl: - symo = rnd(HEADR+segtext.len, INITRND)+rnd(segrodata.len, INITRND)+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; symo = rnd(symo, INITRND); break; case Hwindows: - symo = rnd(HEADR+segtext.filelen, PEFILEALIGN)+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; symo = rnd(symo, PEFILEALIGN); break; } diff --git a/src/cmd/8l/asm.c b/src/cmd/8l/asm.c index 114a3eb5d7..c135dce709 100644 --- a/src/cmd/8l/asm.c +++ b/src/cmd/8l/asm.c @@ -619,17 +619,17 @@ asmb(void) if(iself) goto Elfsym; case Hplan9: - symo = HEADR+segtext.filelen+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; break; case Hdarwin: - symo = rnd(HEADR+segtext.filelen, INITRND)+rnd(segdata.filelen, INITRND)+machlink; + symo = segdata.fileoff+rnd(segdata.filelen, INITRND)+machlink; break; Elfsym: - symo = rnd(HEADR+segtext.filelen, INITRND)+rnd(HEADR+segrodata.filelen, INITRND)+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; symo = rnd(symo, INITRND); break; case Hwindows: - symo = rnd(HEADR+segtext.filelen, PEFILEALIGN)+segdata.filelen; + symo = segdata.fileoff+segdata.filelen; symo = rnd(symo, PEFILEALIGN); break; } -- cgit v1.2.3-54-g00ecf