This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Protect first entries of got.plt with RelRo.
AbandonedPublic

Authored by grimar on May 6 2016, 10:09 AM.

Details

Reviewers
ruiu
rafael
Summary

First entries of got.plt can be protected using RelRo technique,
this patch implements this by shifting got.plt to the memory page,
where relro is active.

Both gold and bfd do that and it should be good for security.

Diff Detail

Event Timeline

grimar updated this revision to Diff 56429.May 6 2016, 10:09 AM
grimar retitled this revision from to [ELF] - Protect first entries of got.plt with RelRo..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: grimar, llvm-commits.
ruiu edited edge metadata.May 6 2016, 10:19 AM

Interesting, but it is tricky. What exactly is the threat for putting the first entry of .got.plt in a writable segment, how realistic is the attack is, and how much does this mitigate the risk?

rafael edited edge metadata.May 6 2016, 11:12 AM
rafael added a subscriber: rafael.

Remind me again how the first entry works. From the ABI it looks like
.got has to have the first entry be the value of _DYNAMIC and the
dynamic linker can use that.

But, it looks like it is actually .got.plt that has that. Also, if the
dynamic linker can find the .got, it can find the .dynamic section,
no?

I see that musl just does

lea _DYNAMIC(%rip),%rsi

to find its own _DYNAMIC.

I wonder if we can just drop the magical first entries.

Cheers,
Rafael

emaste added a subscriber: emaste.May 6 2016, 1:50 PM

If going for safety you should really disable lazy loading or even just compile with -fno-plt.

This patch is not terribly complex, but I also suspect that in practice folks will use -zrelro & -znow if this is a concern.

ELF/OutputSections.h
92

This comment is now confusing to me -- "page aligned" isn't how I'd describe the effect of PageAlignKind::GotPlt. What about just describing these three types where the enum is defined?

grimar abandoned this revision.May 10 2016, 2:59 AM

Well, I assumed it is useful, at least because even linkerscript has special command
DATA_SEGMENT_RELRO_END to do that:

.got            : { *(.got) *(.igot) }
. = DATA_SEGMENT_RELRO_END (24, .);
.got.plt        : { *(.got.plt)  *(.igot.plt) }

I don't know how realistic is attack using unprotected first 3 entries.
Second and third are reserved for dynamic linker use and it is probably would be very specific attack.
So may be use of -zrelro & -znow is enough to protect (since whole .got.plt is covered by relro which is even more safe)
and also avoids additional code complexity.