This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Disable relro when -omagic specified.
ClosedPublic

Authored by grimar on Dec 1 2016, 3:01 AM.

Details

Summary

It was noticed on review page for D26888 (-N) by Ed Maste that we have relro enabled.

Before this patch output from testcase binary was:

Sections:
Idx Name          Size      Address          Type
  0               00000000 0000000000000000
  1 .dynsym       00000048 0000000000200158
  2 .hash         00000020 00000000002001a0
  3 .dynstr       00000021 00000000002001c0
  4 .rela.dyn     00000018 00000000002001e8
  5 .rela.plt     00000018 0000000000200200
  6 .text         0000000a 0000000000200218 TEXT DATA
  7 .plt          00000020 0000000000200230 TEXT DATA
  8 .dynamic      000000f0 0000000000200250
  9 .got          00000008 0000000000200340 DATA
 10 .data         00000008 0000000000201000 DATA
 11 .foo          00000004 0000000000201008 DATA
 12 .got.plt      00000020 0000000000201010 DATA
 13 .comment      00000008 0000000000000000
 14 .symtab       00000060 0000000000000000
 15 .shstrtab     0000007b 0000000000000000
 16 .strtab       00000013 0000000000000000

  ProgramHeader {
    Type: PT_GNU_RELRO (0x6474E552)
    Offset: 0x250
    VirtualAddress: 0x200250
    PhysicalAddress: 0x200250
    FileSize: 248
    MemSize: 248
    Flags [ (0x4)
      PF_R (0x4)
    ]
    Alignment: 1
  }

Noticable here that .data section (first section after relro) is aligned to page.
That is probably not expected behavior with omagic, that says that text and data segments should not be
aligned (I would expect no page alignment for sections either).
Also with omagic we do not page align writable segment, PT_GNU_RELRO starts from 0x200250,
somewhere at the middle of PT_LOAD,
If dynamic linker will apply relro, it probably will round down start address to 200000 to mark it readonly.
Not sure if something bad happens here since sections
there are already non writable ("a", "ax"), but anyways that probably does not look technically correct to do.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 79882.Dec 1 2016, 3:01 AM
grimar retitled this revision from to [ELF] - Disable relro when -omagic specified..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, emaste, evgeny777.
ruiu accepted this revision.Dec 2 2016, 4:20 PM
ruiu edited edge metadata.

LGTM

ELF/Driver.cpp
578 ↗(On Diff #79882)

Please add comments as it's not obvious.

// --omagic is an option to create old-fashioned executables in which
// .text segments are writable. Today, the option is still in use to
// create special-purpose programs such as boot loaders. It doesn't
// make sense to create PT_GNU_RELRO for such executables.
This revision is now accepted and ready to land.Dec 2 2016, 4:20 PM
This revision was automatically updated to reflect the committed changes.