Skip to content

rtld: Tweak handling of mismatched dynamic tags for PLTs #2366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

bsdjhb
Copy link
Collaborator

@bsdjhb bsdjhb commented Mar 13, 2025

With compartmentalized binaries, a compartment might contain a PLT
that only has IPLT stubs. lld still emits DT_ tags for such a PLT,
but the DT_PLTRELSZ tag is zero. To handle these, scan the Plt_Entry
structures after scanning .dynamic and drop entries with zero size.
This also handles the aarch64 special case for a Morello LLD bug.

While here, rework some of the multi-plt handling. Detect mismatched
PLT tags sooner and return false from digest_dynamic1() for a mismatch
and remove sanity checks on PLTs from digest_dynamic2().

While here, fix rtld to die if either digest_dynamic call fails for
rtld itself.

@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 13, 2025

This fixes an issue exposed by the test program with per-function compartments.

@jrtc27
Copy link
Member

jrtc27 commented Mar 13, 2025

What's the actual problem that occurs here though?

@jrtc27
Copy link
Member

jrtc27 commented Mar 13, 2025

technos:cheribsd jrtc4% cat test.c                                                                          
__attribute__((ifunc("foo_resolver")))
static void foo(void);

static void
foo1(void)
{
}

static void
(*foo_resolver(void))(void)
{
        return (&foo1);
}

void
(*get_foo(void))(void) {
        return (&foo);
}

void
_start(void)
{
}
technos:cheribsd jrtc4% clang --target=aarch64-unknown-freebsd test.c -fuse-ld=lld -o - -nostdlib | llvm-readelf -WSsrd -
There are 12 section headers, starting at offset 0x4c0:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .interp           PROGBITS        00000000002001c8 0001c8 000015 00   A  0   0  1
  [ 2] .rela.dyn         RELA            00000000002001e0 0001e0 000018 18  AI  0   7  8
  [ 3] .eh_frame_hdr     PROGBITS        00000000002001f8 0001f8 00002c 00   A  0   0  4
  [ 4] .eh_frame         PROGBITS        0000000000200228 000228 00007c 00   A  0   0  8
  [ 5] .text             PROGBITS        00000000002102a4 0002a4 000020 00  AX  0   0  4
  [ 6] .iplt             PROGBITS        00000000002102d0 0002d0 000010 00  AX  0   0 16
  [ 7] .got.plt          PROGBITS        00000000002202e0 0002e0 000008 00  WA  0   0  8
  [ 8] .comment          PROGBITS        0000000000000000 0002e8 000042 01  MS  0   0  1
  [ 9] .symtab           SYMTAB          0000000000000000 000330 0000f0 18     11   7  8
  [10] .shstrtab         STRTAB          0000000000000000 000420 000063 00      0   0  1
  [11] .strtab           STRTAB          0000000000000000 000483 00003c 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  R (retain), p (processor specific)

Relocation section '.rela.dyn' at offset 0x1e0 contains 1 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
00000000002202e0  0000000000000408 R_AARCH64_IRELATIVE               2102a4

Symbol table '.symtab' contains 10 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT   ABS test.c
     2: 00000000002102a4    12 FUNC    LOCAL  DEFAULT     5 foo_resolver
     3: 00000000002102a4     0 NOTYPE  LOCAL  DEFAULT     5 $x.0
     4: 00000000002102c0     4 FUNC    LOCAL  DEFAULT     5 foo1
     5: 0000000000000027     0 NOTYPE  LOCAL  DEFAULT     8 $d.1
     6: 0000000000200228     0 NOTYPE  LOCAL  DEFAULT     4 $d.2
     7: 00000000002102b0    12 FUNC    GLOBAL DEFAULT     5 get_foo
     8: 00000000002102d0     0 FUNC    GLOBAL DEFAULT     6 foo
     9: 00000000002102bc     4 FUNC    GLOBAL DEFAULT     5 _start

If it's that a binary with .iplt but no .plt has a .got.plt that isn't big enough (i.e. it omits the real .got.plt but includes the IPLT's .got.plt), that exists as an upstream LLVM bug already (though that's using the system LLVM 14 so I guess could be fixed). I'd rather just see LLVM be fixed to not do broken things like that.

@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 13, 2025

I have compartments that have a PLT with only an IPLT. The IPLT entries were for cross-compartment calls to static functions (in this case each function is in its own compartment including static functions). rtld assumed that there would be the requisite GOT entries at the start for use by PLT0, but without an actual PLT these weren't present and rtld ended up stomping on some of the IGOTPLT entries instead when it tried to write out these initial entries.

The other thing I've found that I'm fixing on the lld side is that I didn't need per-compartment rela.iplt sections in the first place, they really should just be part of rela.dyn. Because of one mistake I made, they ended up in the per-compartment rela.plt, but once I fixed the logic I had wrong there, I ended up with per-compartment rela.dyn that weren't covered by any DT_* tag, so then I reverted to a global relaIplt (that is merged into rela.dyn) and things are much simpler. Hmm, and with that fixed, I also no longer have these empty PLTRELSZ entries anymore in my test case (or at least, now you can only get them for the default compartment if it has no real PLT entries, only IPLT entries).

I do think the way I'm handling the counting mismatches for the PLT-related DT_* tags is better this way, and it's still true that Morello lld had a bug in older versions that would omit an empty PLTREL that this handles.

@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 13, 2025

But it's true I might need to rewrite the commit message at least.

Detect mismatched PLT tags sooner and return false from
digest_dynamic1() for a mismatch and remove sanity checks on PLTs from
digest_dynamic2().

While here, fix rtld to die if either digest_dynamic call fails for
rtld itself.
@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 13, 2025

Ok, this is now just a cosmetic cleanup and is no longer a required "fix" for anything. My aforementioned changes to collapse relaIplt down into the global rela.dyn has been the real fix for the issue I was seeing originally.

@bsdjhb bsdjhb changed the title rtld: Handle PLTs that only contain IPLT stubs rtld: Tweak handling of mismatched dynamic tags for PLTs Mar 13, 2025
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 18, 2025
Libarchive 3.7.7

Security fixes:
 CTSRD-CHERI#2158 rpm: calculate huge header sizes correctly
 CTSRD-CHERI#2160 util: fix out of boundary access in mktemp functions
 CTSRD-CHERI#2168 uu: stop processing if lines are too long
 CTSRD-CHERI#2174 lzop: prevent integer overflow
 CTSRD-CHERI#2172 rar4: protect copy_from_lzss_window_to_unp() (CVE-2024-20696)
 CTSRD-CHERI#2175 unzip: unify EOF handling
 CTSRD-CHERI#2179 rar4: fix out of boundary access with large files
 CTSRD-CHERI#2203 rar4: fix OOB access with unicode filenames
 CTSRD-CHERI#2210 rar4: add boundary checks to rgb filter
 CTSRD-CHERI#2248 rar4: fix OOB in delta filter
 CTSRD-CHERI#2249 rar4: fix OOB in audio filter
 CTSRD-CHERI#2256 fix multiple vulnerabilities identified by SAST
 CTSRD-CHERI#2258 cpio: ignore out-of-range gid/uid/size/ino and harden AFIO parsing
 CTSRD-CHERI#2265 rar5: clear 'data ready' cache on window buffer reallocs
 CTSRD-CHERI#2269 rar4: fix CVE-2024-26256 (CVE-2024-26256)
 CTSRD-CHERI#2330 iso: be more cautious about parsing ISO-9660 timestamps
 CTSRD-CHERI#2343 tar: clean up linkpath between entries
 CTSRD-CHERI#2364 tar: don't crash on truncated tar archives
 CTSRD-CHERI#2366 gzip: prevent a hang when processing a malformed gzip inside a gzip
 CTSRD-CHERI#2377 tar: fix two leaks in tar header parsing

Important bugfixes:
 CTSRD-CHERI#2096 rar5: report encrypted entries
 CTSRD-CHERI#2150 xar: fix another infinite loop and expat error handling
 CTSRD-CHERI#2173 shar: check strdup return value
 CTSRD-CHERI#2161 lha: fix integer truncation on 32-bit systems
 CTSRD-CHERI#2338 tar: fix memory leaks when processing symlinks or parsing pax headers
 CTSRD-CHERI#2245 7zip: fix issue when skipping first file in 7zip archive that
       is a multiple of 65536 bytes
 CTSRD-CHERI#2252 7-zip: read/write symlink paths as UTF-8
 CTSRD-CHERI#2259 rar5: don't try to read rediculously long names
 CTSRD-CHERI#2290 ar: fix archive entries having no type
 CTSRD-CHERI#2360 tar: fix truncation of entry pathnames in specific archives

CVE:		CVE-2024-20696, CVE-2024-26256
PR:		282047 (exp-run)
MFC after:	1 week
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 18, 2025
Libarchive 3.7.7

Security fixes:
 CTSRD-CHERI#2158 rpm: calculate huge header sizes correctly
 CTSRD-CHERI#2160 util: fix out of boundary access in mktemp functions
 CTSRD-CHERI#2168 uu: stop processing if lines are too long
 CTSRD-CHERI#2174 lzop: prevent integer overflow
 CTSRD-CHERI#2172 rar4: protect copy_from_lzss_window_to_unp() (CVE-2024-20696)
 CTSRD-CHERI#2175 unzip: unify EOF handling
 CTSRD-CHERI#2179 rar4: fix out of boundary access with large files
 CTSRD-CHERI#2203 rar4: fix OOB access with unicode filenames
 CTSRD-CHERI#2210 rar4: add boundary checks to rgb filter
 CTSRD-CHERI#2248 rar4: fix OOB in delta filter
 CTSRD-CHERI#2249 rar4: fix OOB in audio filter
 CTSRD-CHERI#2256 fix multiple vulnerabilities identified by SAST
 CTSRD-CHERI#2258 cpio: ignore out-of-range gid/uid/size/ino and harden AFIO parsing
 CTSRD-CHERI#2265 rar5: clear 'data ready' cache on window buffer reallocs
 CTSRD-CHERI#2269 rar4: fix CVE-2024-26256 (CVE-2024-26256)
 CTSRD-CHERI#2330 iso: be more cautious about parsing ISO-9660 timestamps
 CTSRD-CHERI#2343 tar: clean up linkpath between entries
 CTSRD-CHERI#2364 tar: don't crash on truncated tar archives
 CTSRD-CHERI#2366 gzip: prevent a hang when processing a malformed gzip inside a gzip
 CTSRD-CHERI#2377 tar: fix two leaks in tar header parsing

Important bugfixes:
 CTSRD-CHERI#2096 rar5: report encrypted entries
 CTSRD-CHERI#2150 xar: fix another infinite loop and expat error handling
 CTSRD-CHERI#2173 shar: check strdup return value
 CTSRD-CHERI#2161 lha: fix integer truncation on 32-bit systems
 CTSRD-CHERI#2338 tar: fix memory leaks when processing symlinks or parsing pax headers
 CTSRD-CHERI#2245 7zip: fix issue when skipping first file in 7zip archive that
       is a multiple of 65536 bytes
 CTSRD-CHERI#2252 7-zip: read/write symlink paths as UTF-8
 CTSRD-CHERI#2259 rar5: don't try to read rediculously long names
 CTSRD-CHERI#2290 ar: fix archive entries having no type
 CTSRD-CHERI#2360 tar: fix truncation of entry pathnames in specific archives

CVE:		CVE-2024-20696, CVE-2024-26256
PR:		282047 (exp-run)
MFC after:	1 week
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants