Skip to content

Commit

Permalink
Fix bug in file shifting that could cause conflicting PT_LOAD segments
Browse files Browse the repository at this point in the history
When a section in the file needs to be enlarged (e.g. to accommodate
setting a larger RPATH), shiftFile() is used to shift all content
following the growing section to a later position in the file.

Commit 109b771 introduced logic to
ensure that, after the segment split, no sections span multiple
segments. This is done by sliding the portion of the segment after the
split point later in the file, then adding a new PT_LOAD segment that
contains the preceding data plus the extra room that is being added. The
existing implementation does this by simply adding
`extraPages*getPageSize()` bytes to the number of bytes ahead of the
split point in the segment.

However, this approach can result in two PT_LOAD segments that overlap
when page boundaries are taken into account. As an example, this PT_LOAD
section (taken from a Python 3.10 binary):

LOAD           0x0000000000000000 0x0000000000400000 0x0000000000400000
               0x0000000000000948 0x0000000000000948  R E    0x200000

is split into the following two sections:

LOAD           0x0000000000000000 0x00000000003ff000 0x00000000003ff000
               0x0000000000001594 0x0000000000001594  R E    0x1000
LOAD           0x0000000000001594 0x0000000000400594 0x0000000000400594
               0x00000000000003b4 0x00000000000003b4  R E    0x1000

Note that the two PT_LOAD sections both contain the memory page at
address 0x400000. The Linux kernel's ELF loader (at least as of v4.18)
does not accept this as a valid ELF executable, triggering a segfault
with si_code=SI_KERNEL immediately when the binary is executed.

The fix here is to set the length of the segment that comes before the
split point more carefully; instead of adding `extraPages*getPageSize()`
bytes to the portion of the segment that came before the split, the
actual number of padding bytes that were needed (before rounding up to
the next multiple of the page size) are used. This avoids the overlap
in the PT_LOAD segments and makes the output files executable again.
  • Loading branch information
Jason committed Dec 2, 2022
1 parent f4f1848 commit 8d2cb4f
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
10 changes: 6 additions & 4 deletions src/patchelf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ static uint64_t roundUp(uint64_t n, uint64_t m)


template<ElfFileParams>
void ElfFile<ElfFileParamNames>::shiftFile(unsigned int extraPages, size_t startOffset)
void ElfFile<ElfFileParamNames>::shiftFile(unsigned int extraPages, size_t startOffset, size_t extraBytes)
{
assert(startOffset >= sizeof(Elf_Ehdr));

Expand Down Expand Up @@ -516,7 +516,7 @@ void ElfFile<ElfFileParamNames>::shiftFile(unsigned int extraPages, size_t start
wri(phdr.p_offset, phdrs.at(splitIndex).p_offset - splitShift - shift);
wri(phdr.p_paddr, phdrs.at(splitIndex).p_paddr - splitShift - shift);
wri(phdr.p_vaddr, phdrs.at(splitIndex).p_vaddr - splitShift - shift);
wri(phdr.p_filesz, wri(phdr.p_memsz, splitShift + shift));
wri(phdr.p_filesz, wri(phdr.p_memsz, splitShift + extraBytes));
wri(phdr.p_flags, splitFlags);
wri(phdr.p_align, getPageSize());
}
Expand Down Expand Up @@ -916,12 +916,14 @@ void ElfFile<ElfFileParamNames>::rewriteSectionsExecutable()
neededSpace += sizeof(Elf_Phdr);
debug("needed space is %d\n", neededSpace);

unsigned int neededPages = roundUp(neededSpace - startOffset, getPageSize()) / getPageSize();
/* Calculate how many bytes are needed out of the additional pages. */
size_t extraSpace = neededSpace - startOffset;
unsigned int neededPages = roundUp(extraSpace, getPageSize()) / getPageSize();
debug("needed pages is %d\n", neededPages);
if (neededPages * getPageSize() > firstPage)
error("virtual address space underrun!");

shiftFile(neededPages, startOffset);
shiftFile(neededPages, startOffset, extraSpace);

firstPage -= neededPages * getPageSize();
startOffset += neededPages * getPageSize();
Expand Down
2 changes: 1 addition & 1 deletion src/patchelf.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class ElfFile

void sortShdrs();

void shiftFile(unsigned int extraPages, size_t sizeOffset);
void shiftFile(unsigned int extraPages, size_t sizeOffset, size_t extraBytes);

std::string getSectionName(const Elf_Shdr & shdr) const;

Expand Down

0 comments on commit 8d2cb4f

Please sign in to comment.