[Bug dynamic-link/31076] Extra struct vm_area_struct with ---p created when PAGE_SIZE < max-page-size

["i at maskray dot me" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=31076

--- Comment #17 from Fangrui Song <i at maskray dot me> ---
Let's refer to the first two build/map maps in comment 0 for discussion.

    # In another console, run
     $ cat /proc/1717808/maps
    564d8e90f000-564d8e910000 r--p 00000000 08:05 2519504       
/sample/build/main
    564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504       
/sample/build/main
    ...

We have three choices for the gap.

1. Do not create a map for the gap (separate mmap using MAP_FIXED, or a large
mmap with explicit munmap).

Linux kernel fs/binfmt_elf.c employs this approach. However, there is a risk
that an unrelated future mmap may be placed in the gap:

    564d8e90f000-564d8e910000 r--p 00000000 08:05 2519504       
/sample/build/main
       ================ an unrelated mmap may be placed in the gap
    564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504       
/sample/build/main

If there is no map in between, there is nothing lose. Accessing the gap will
cause SIGSEGV.

Is the potential occurrence of an unrelated mmap considered a compromise in the
hardening features?
Personally, I don't think this poses a significant issue. The program does not
access the gaps.
We can even guarantee this property for direct access when input relocations to
the linker use symbols with in-bounds addends (e.g. when x is defined relative
to an input section, we know R_X86_64_PC32(x) must be in-bounds).

However, some programs may expect contiguous maps of a file (such as when glibc
link_map::l_contiguous is set to 1).
Does this choice render the program exploitable if an attacker can ensure a map
within the gap instead of outside the file?
It seems to me that they could achieve everything with a map outside of the
file.

Having said that, the presence of an unrelated map between maps associated with
a single file descriptor remains odd, so it's preferable to avoid it if
possible (Choice 3 below).

2. Map the gap with the associated fd

This approach is commonly adopted by many dynamic loaders.
It involves a large PROT_READ mmap covering the whole file followed by several
mmap instances using MAP_FIXED and the relevant flags.

The gaps may have permissions set as r--p or ---p, resulting in additional
instances of the `struct vm_area_struct`.
Within glibc/elf/dl-map-segments.h, the `has_holes` code calls mprotect to
change r--p permissions to ---p:

    564d8e90f000-564d8e910000 r--p 00000000 08:05 2519504       
/sample/build/main
    564d8e910000-564d8e91f000 ---p 00001000 08:05 2519504       
/sample/build/main
    564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504       
/sample/build/main

Although ---p might be seen as a hardening measure, personally, I don't believe
it significantly impacts the exploitability.
While there might be plenty of gadgets in r-xp sections, reducing gadgets in
r--p sections doesn't seem notably useful.
(https://isopenbsdsecu.re/mitigations/rop_removal/)

3. Extend the map end to cover the gap

    564d8e90f000-**564d8e91f000** r--p 00000000 08:05 2519504       
/sample/build/main  (the end is extended)
    564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504       
/sample/build/main

The loader code can adjust p_filesz and p_memsz when invoking mmap.

By extending the map size to a multiple of the maximum page size, it benefits
transparent huge pages or a userspace hugepage remapper.

Right now Android Bionic is exploring this choice:
https://r.android.com/2796855

---

Personally I favor "3. Extend the map end to cover the gap" the most.
I've also pondered whether this falls under the purview of linkers. Such a
change seems intrusive and unsightly.
If the linker extends the end of p_memsz to cover the gap, should it also
extend p_filesz?

* If it doesn't, we create a PT_LOAD with p_filesz/p_memsz that is not for BSS,
which is weird.
* If it does, we have an output file featuring overlapping file offset ranges,
which is weird as well.

Moreover, a PT_LOAD whose end isn't backed by a section is unusual. I'm
concerned that many binary manipulation tools may not handle this case
correctly.
Utilizing a linker script can intentionally create discontiguous address
ranges. I'm concerned that the linker might not discern such cases with
intelligent logic regarding p_filesz/p_memsz.

This feature request seems to be within the realm of loaders and specific
information, such as the page size, is only accessible to loaders.
I believe loaders are better equipped to handle this task."

-- 
You are receiving this mail because:
You are on the CC list for the bug.