RELRO not working on Ubuntu 14.04

Bug #1412553 reported by bugproxy on 2015-01-19
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
binutils (Ubuntu)
Undecided
Taco Screen team
Trusty
Undecided
Unassigned

Bug Description

RELRO (RELocation Read-Only) is security feature provided by the linker and implemented in conjunction with glibc to relocations sections that are used to resolve dynamically loaded functions read-only.

The GLIBC side looks at the segments placement set by the linker and if they met some criteria regarding alignment, the are mprotect to be read-only. More specifically, on GLIBC code:

elf/dl-reloc.c:

324 void internal_function
325 _dl_protect_relro (struct link_map *l)
326 {
327 ElfW(Addr) start = ((l->l_addr + l->l_relro_addr)
328 & ~(GLRO(dl_pagesize) - 1));
329 ElfW(Addr) end = ((l->l_addr + l->l_relro_addr + l->l_relro_size)
330 & ~(GLRO(dl_pagesize) - 1));
331
332 if (start != end
333 && __mprotect ((void *) start, end - start, PROT_READ) < 0)
334 {
335 static const char errstring[] = N_("\
336 cannot apply additional memory protection after relocation");
337 _dl_signal_error (errno, l->l_name, NULL, errstring);
338 }
339 }

The problem is, if linker does not set the alignment correctly, 'start' and 'end' will be equal and thus not protected. And this is happening on Ubuntu 14.04 due the fact its uses default binutils elf{32,64}-ppc.c ELF_COMMONPAGESIZE to align it to 4k instead of 64k.

There is a recent patch on binutils-dev maillist [1] to change the default for 64k and Fedora rawhide already sets it [2] in its binutils.spec spec:

# On ppc64 and aarch64, we might use 64KiB pages
sed -i -e '/#define.*ELF_COMMONPAGESIZE/s/0x1000$/0x10000/' bfd/elf*ppc.c
sed -i -e '/#define.*ELF_COMMONPAGESIZE/s/0x1000$/0x10000/' bfd/elf*aarch64.c

Ubuntu for powerpc64le should do the same.

[1] https://sourceware.org/ml/binutils/2014-12/msg00165.html
[2] http://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/source/SRPMS/b/binutils-2.24-29.fc22.src.rpm

I tested and building a binutils with the ELF_COMMONPAGESIZE set to 64k instead of default 4k and rebuilding GLIBC I see its relocation sections being correctly mprotected.

bugproxy (bugproxy) on 2015-01-19
tags: added: architecture-ppc64le bugnameltc-119848 severity-high targetmilestone-inin---
affects: ubuntu → glibc (Ubuntu)
Changed in glibc (Ubuntu):
status: New → Confirmed
assignee: nobody → Taco Screen team (taco-screen-team)
bugproxy (bugproxy) on 2015-01-19
tags: added: targetmilestone-inin14042
removed: targetmilestone-inin---
affects: glibc (Ubuntu) → binutils (Ubuntu)
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package binutils - 2.25-2ubuntu2

---------------
binutils (2.25-2ubuntu2) vivid; urgency=medium

  * Use 64k for COMMONPAGESIZE on powerpc, ppc64 and ppc64el. LP: #1412553.
 -- Matthias Klose <email address hidden> Fri, 23 Jan 2015 14:13:08 +0100

Changed in binutils (Ubuntu):
status: Confirmed → Fix Released

------- Comment From <email address hidden> 2015-01-26 15:21 EDT-------
*** Bug 119945 has been marked as a duplicate of this bug. ***

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2015-01-26 16:02 EDT-------
This is a comment from Alan Modra:

COMMONPAGESIZE in the linker should match kernel page size for two reasons:
- The linker optimisation that trades disk pages for memory pages is most
effective when they match. If you're running a kernel that, say, uses 64k
pages, but your linker commonpagesize is 4k then your executables and
shared libraries will use up to 4k extra on disk but be ineffective in
saving memory pages. Only 1 in 16 binaries will save a memory page.
Conversely, if your kernel used 4k pages but commonpagesize was 16k, then
you'd waste up to 16k on disk to make only a 4k saving in memory, ie. you'd
be wasting 12k of disk to no purpose.
- The linker -z relro layout also uses commonpagesize. Some might say this
is a bug.. -z relro also increases executable size, both in memory and on
disk, in order to reduce the potential for an evil attacker to subvert a
binary, by making more of the binary read-only at startup. Like the
disk/memory tradeoff, -z relro is ineffective if commonpagesize is
*smaller* than the actual kernel page size. (The same 1 in n binaries,
where n is commonpagesize/kernelpagesize, will properly protect the last
page in the relro area.) If commonpagesize is larger than the kernel page
size then -z relro works but wastes memory.

So that's the WHY. Sorry it isn't complete as I don't know off the top of
my head what page size the Ubuntu kernels use.

Note that from the -z relro view, we have other things that need fixing in
the linker but I wouldn't recommend backporting just yet. The patches I've
committed upstream 3e2b0f31, 23283c1b, 5ad18f16, aren't too well tested and
are known to break some xlc compiled binaries.

Breno Leitão (breno-leitao) wrote :

Any idea on when this patch will be accepted in 14.04?

Hello bugproxy, or anyone else affected,

Accepted binutils into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/binutils/2.24-5ubuntu13 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in binutils (Ubuntu Trusty):
status: New → Fix Committed
tags: added: verification-needed
tags: added: verification-done
removed: verification-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package binutils - 2.24-5ubuntu13

---------------
binutils (2.24-5ubuntu13) trusty-proposed; urgency=medium

  * SRU LP: #1311866.
  * Update from the 2.24 branch, 20141113.
    - Fix PR gold/16945, properly handle 64-bit GOT relocations on x86_64.
    - Fix PR gold/16900, an issue where first reserved word of GOT is not
      initialized if there is no PLT.
    - gold: Fix handling of __ehdr_start when it cannot be defined.
    - Apply mainline patches for ppc476 workaround.
    - Add binutils test cases for AArch64.
    - Disassembler fix on AArch64.
    - Fix PR ld/17047, crash in the bfd linker with MALLOC_PERTURB.
    - Fix PR ld/17277, ARM32, bogus DT_TEXTREL marker (and R_ARM_NONE)
      for PC-relative cross-section relocs.
    - [AArch64] Cortex-A53 erratum 835769 linker workaround.
  * Remove the aarch64-fix-instruction-mask, applied on the branch.
  * Fix PR gold/15639, -flto and ld.gold on ARM. LP: #1191909.
  * Use 64k for COMMONPAGESIZE on PPC. LP: #1412553.
  * Fix PR ld/16452, PR ld/16457, don't output symbol version definitions
    for non-DT_NEEDED. LP: #1248642.
  * Add powerpc target for ppc64el builds. Closes: #760395. LP: #1433238.
  * binutils-doc: Include all info files. LP: #1410780.
  * Fix PR ld/16715 (ARM), set st_value to zero for undefined symbols.
    LP: #1441961.
 -- Matthias Klose <email address hidden> Tue, 14 Apr 2015 19:52:45 +0200

Changed in binutils (Ubuntu Trusty):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for binutils has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers