From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id DC3F338582BE; Sat, 2 Dec 2023 17:08:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DC3F338582BE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1701536895; bh=4dOMyy5iLhEYHlgpYdcWK3NsozW7L3Hzwr/BOFzWxno=; h=From:To:Subject:Date:In-Reply-To:References:From; b=oD5IuS9Nb8NhgRRGxlg5U9eblc8rVwEZ31XVH6YzqAyWqjT1X9qsGngTsvmoCvxOK 6MpvlY+38qPxVrntk+IXAaKyJvGzT568CvZEMEfFqPHNP1ruPXbve4+MqHcee/EuKJ 2ovIwfzVLNzwy+pTssvCw0v27122KoD9sgP5di3Y= From: "i at maskray dot me" To: glibc-bugs@sourceware.org Subject: [Bug dynamic-link/31076] Extra struct vm_area_struct with ---p created when PAGE_SIZE < max-page-size Date: Sat, 02 Dec 2023 17:08:12 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: glibc X-Bugzilla-Component: dynamic-link X-Bugzilla-Version: unspecified X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: i at maskray dot me X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: unassigned at sourceware dot org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://sourceware.org/bugzilla/show_bug.cgi?id=3D31076 --- Comment #17 from Fangrui Song --- 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=20=20=20=20=20=20= =20 /sample/build/main 564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504=20=20=20=20=20=20= =20 /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 lar= ge 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=20=20=20=20=20=20= =20 /sample/build/main =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D an unrelated mmap m= ay be placed in the gap 564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504=20=20=20=20=20=20= =20 /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 relocation= s to the linker use symbols with in-bounds addends (e.g. when x is defined relat= ive 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 g= libc 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 seve= ral 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=20=20=20=20=20=20= =20 /sample/build/main 564d8e910000-564d8e91f000 ---p 00001000 08:05 2519504=20=20=20=20=20=20= =20 /sample/build/main 564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504=20=20=20=20=20=20= =20 /sample/build/main Although ---p might be seen as a hardening measure, personally, I don't bel= ieve 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=20=20=20=20= =20=20=20 /sample/build/main (the end is extended) 564d8e91f000-564d8e920000 r-xp 00010000 08:05 2519504=20=20=20=20=20=20= =20 /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 benefi= ts 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 rang= es, 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." --=20 You are receiving this mail because: You are on the CC list for the bug.=