1388713 - lld - dummy binary with the elfhack segsfaults

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

Attachment: core

Thanks to bug 1385783, the hack no longer fails.

This command is segfaulting:
$ LD_PRELOAD=/data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack/test-array.so  /data/jenkins/workspace/firefox-clang-lld-last/obj-x86_64-pc-linux-gnu/build/unix/elfhack/dummy
Segmentation fault (core dumped)

core dump as attachment (debian 64b)

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-array.so

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: dummy

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1388713 - Change how elfhack looks for the bss section.

In bug 635961, elfhack was made to (ab)use the bss section as a
temporary space for a pointer. To find it, it scanned writable PT_LOAD
segments to find one that has a different file and memory size,
indicating the presence of .bss. This usually works fine, but when
the binary is linked with lld and relro is enabled, the end of the
file-backed part of the PT_LOAD segment containing the .bss section
ends up in the RELRO segment, making that location read-only and
subsequently making the elfhacked binary crash when it tries to restore
the .bss to a clean state, because it's not actually writing in the .bss
section: lld page aligns it after the RELRO segment.

So instead of scanning PT_LOAD segments, we scan for SHT_NOBITS
sections that are not SHF_TLS (i.e. not .tbss).

Review commit: https://reviewboard.mozilla.org/r/166790/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166790/

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8895586 [details]
Bug 1388713 - Change how elfhack looks for the bss section.

https://reviewboard.mozilla.org/r/166790/#review172456

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0417716bac0
Change how elfhack looks for the bss section. r=froydnj

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e0417716bac0

Comment 7

[Sylvestre Ledru [:Sylvestre]]

I am still seeing this issue

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Attachment: core

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Attachment: dummy

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-array.so

Comment 11

[Mike Hommey [:glandium]]

That new test-array.so is impossible considering the current code. Are you sure you built a version that had this landed?

Comment 12

[Mike Hommey [:glandium]]

Specifically, it has exactly the same problem as the original one, with the mprotect pointer pointing before the bss. There is *no way* the new code can get that address, because there is no section for it.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

I am building this one:

changeset:   374096:65826179c86e
tag:         tip
fxtree:      central
user:        Jonathan Kew <[e-mail]>
date:        Fri Aug 11 11:12:12 2017 +0100
summary:     Bug 1387912 - Add missing #include to dom/canvas/ImageBitmap.h, to 

I re triggered a new build to be sure.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Same, full log:
http://relman-ci.mozilla.org/job/firefox-clang-lld-last/34/consoleFull

Comment 15

[Mike Hommey [:glandium]]

Please attach the non-elfhacked test-array.so.

Comment 16

[Mike Hommey [:glandium]]

That is, test-array.so.bak

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Attachment: test-array.so.bak

voila!

Comment 18

[Mike Hommey [:glandium]]

Well, when I take that test-array.so.bak and apply a locally built elfhack to it, I get the mprotect relocation at the right place:

00000000c000  000b00000001 R_X86_64_64       0000000000000000 mprotect + 0

While the test-array.so you attached has:

00000000b1c8  000b00000001 R_X86_64_64       0000000000000000 mprotect + 0

I see no way elfhack can produce the latter from that test-array.so.bak. I *do* get the latter if I locally backout the patch for this bug. Are you absolutely sure you're building the right thing?

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Looks like it is different issue and the test-array.so is different, opening a new bug