Skip to content

Fix collision detection for resized PHTs - #644

Closed
godlygeek wants to merge 2 commits into
NixOS:masterfrom
godlygeek:fix_pht_collision_detection
Closed

Fix collision detection for resized PHTs#644
godlygeek wants to merge 2 commits into
NixOS:masterfrom
godlygeek:fix_pht_collision_detection

Conversation

@godlygeek

Copy link
Copy Markdown
Contributor

Patching a shared library might require extending its program header
table, which can then cause it to run out of its originally allocated
space. After 484d349, patchelf handles this by detecting that growing
the PHT in place would result in a collision with a section that won't
be moved, in which case it instead moves the PHT to the end of the ELF
file.

The logic checking for collisions incorrectly assumes that the PHT is
originally at the start of the file, though. It checks every section
after the first to see if it overlaps with the PHT given its new size,
but it assumes that the PHT's original position is immediately after the
ELF header, sizeof(Elf_Ehdr) bytes into the file.

In other words, it's currently looking for collisions in the range:

[0, roundUp(sizeof(Elf_Ehdr) + newPhtSize, sectionAlignment)]

This commit fixes it to check for collisions in the correct range:

[hdr()->e_phoff, hdr()->e_phoff + roundUp(newPhtSize, sectionAlignment)]

Closes #643

@godlygeek
godlygeek force-pushed the fix_pht_collision_detection branch from 5023ffc to cfc7c2d Compare June 17, 2026 19:20
Add a regression test to ensure that PHT collision detection correctly
detects when growing the PHT in place would cause it to collide with
another section, even if the PHT is not at the start of the ELF file.
Patching a shared library might require extending its program header
table, which can then cause it to run out of its originally allocated
space. After 484d349, patchelf handles this by detecting that growing
the PHT in place would result in a collision with a section that won't
be moved, in which case it instead moves the PHT to the end of the ELF
file.

The logic checking for collisions incorrectly assumes that the PHT is
originally at the start of the file, though. It checks every section
after the first to see if it overlaps with the PHT given its new size,
but it assumes that the PHT's original position is immediately after the
ELF header, `sizeof(Elf_Ehdr)` bytes into the file.

In other words, it's currently looking for collisions in the range:

    [0, roundUp(sizeof(Elf_Ehdr) + newPhtSize, sectionAlignment)]

This commit fixes it to check for collisions in the correct range:

    [hdr()->e_phoff, hdr()->e_phoff + roundUp(newPhtSize, sectionAlignment)]
@godlygeek
godlygeek force-pushed the fix_pht_collision_detection branch from cfc7c2d to 6cd7c9a Compare June 17, 2026 19:30
@godlygeek

Copy link
Copy Markdown
Contributor Author

It was surprisingly difficult to write a regression test for this. I'd be open to doing it a different way - either vendoring a .so where the PHT is in the middle of the ELF file instead of the start, or doing something like repeatedly calling patchelf to modify the same ELF file until it relocates the PHT, and then making sure that the N+1th modification doesn't overwrite anything.

@domenkozar

Copy link
Copy Markdown
Member

Fixed in #652

@domenkozar domenkozar closed this Jun 26, 2026
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.

patchelf fails to detect it must relocate the PHT when it is already not at the beginning of the file

2 participants