[PATCH] D43819: [ELF] - Restrict section offsets that exceeds file size.

[Rafael Ávila de Espíndola via Phabricator via llvm-commits]

rafael added a comment.

LGTM

It feels like we should be able to error earlier, but it is better to
fix the crash first and investigate alternatives second.

Cheers,
Rafael

George Rimar via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> grimar updated this revision to Diff 136725.
>  grimar added a comment.
> 
> - Addressed review comments.
> 
> https://reviews.llvm.org/D43819
> 
> Files:
> 
>   ELF/Writer.cpp
>   test/ELF/linkerscript/Inputs/sections-va-overflow.s
>   test/ELF/linkerscript/sections-va-overflow.test
> 
> Index: test/ELF/linkerscript/sections-va-overflow.test
>  ===================================================================
> 
>   - test/ELF/linkerscript/sections-va-overflow.test +++ test/ELF/linkerscript/sections-va-overflow.test @@ -0,0 +1,21 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/sections-va-overflow.s -o %t.o +# RUN: not ld.lld -o /dev/null --script %s %t.o 2>&1 | FileCheck %s -check-prefix=ERR + +PHDRS { +  ph_headers  PT_PHDR PHDRS; +  ph_text PT_LOAD FILEHDR PHDRS  FLAGS (0x1 | 0x4); +} +SECTIONS { + . = 0xffffffff20000000; + .text : { *(.text*) } : ph_text + .test 0x1000 : { BYTE(0) } + .bss : { *(.bss*) } +} + +## Section .test has VA 0x1000 and placed in the same segment as section .text +## with VA 0xffffffff20000000. That might be technically correct, but most probably +## is a result of a broken script file and causes file offset calculation overflow. +## It seems we do not have to support it, so we don't and we report an error in this case. +# ERR: error: unable to place section .text at file offset [0xFFFFFFFF20000000 -> 0xFFFFFFFE40000000]; check your linker script for overflows +# ERR-NOT: unable to place section .bss Index: test/ELF/linkerscript/Inputs/sections-va-overflow.s ===================================================================
>   - test/ELF/linkerscript/Inputs/sections-va-overflow.s +++ test/ELF/linkerscript/Inputs/sections-va-overflow.s @@ -0,0 +1,6 @@ +.global _start +_start: +  retq + +.bss +.space 0x2000 Index: ELF/Writer.cpp ===================================================================
>   - ELF/Writer.cpp +++ ELF/Writer.cpp @@ -1826,6 +1826,12 @@ } }
> 
>     +static std::string rangeToString(uint64_t Addr, uint64_t Len) { +  if (Len == 0) +    return "<empty range at 0x" + utohexstr(Addr) + ">"; +  return "[0x" + utohexstr(Addr) + " -> 0x" + utohexstr(Addr + Len - 1) + "]"; +} + // Adjusts the file alignment for a given output section and returns // its new file offset. The file offset must be the same with its // virtual address (modulo the page size) so that the loader can load @@ -1896,6 +1902,18 @@
> 
>     SectionHeaderOff = alignTo(Off, Config->Wordsize); FileSize = SectionHeaderOff + (OutputSections.size() + 1) * sizeof(Elf_Shdr); + +  // It is possible to get file offset overlaps or overflows with linker +  // scripts. We perform checks required in checkNoOverlappingSections() and +  // want to prevent file size overflows here because it would crash the linker. +  for (OutputSection *Sec : OutputSections) { +    if (Sec->Type == SHT_NOBITS) +      continue; +    if ((Sec->Offset > FileSize) || (Sec->Offset + Sec->Size > FileSize)) +      error("unable to place section " + Sec->Name + " at file offset " + +            rangeToString(Sec->Offset, Sec->Offset + Sec->Size) + +            "; check your linker script for overflows"); +  } }
> 
>     // Finalize the program headers. We call this function after we assign @@ -1934,12 +1952,6 @@ } }
> 
>     -static std::string rangeToString(uint64_t Addr, uint64_t Len) {
> - if (Len == 0)
> - return "<empty range at 0x" + utohexstr(Addr) + ">";
> - return "[0x" + utohexstr(Addr) + " -> 0x" + utohexstr(Addr + Len - 1) + "]"; -} - // Check whether sections overlap for a specific address range (file offsets, // load and virtual adresses). //
> 
> Index: test/ELF/linkerscript/sections-va-overflow.test
>  ===================================================================
> 
>   - test/ELF/linkerscript/sections-va-overflow.test +++ test/ELF/linkerscript/sections-va-overflow.test @@ -0,0 +1,21 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/sections-va-overflow.s -o %t.o +# RUN: not ld.lld -o /dev/null --script %s %t.o 2>&1 | FileCheck %s -check-prefix=ERR + +PHDRS { +  ph_headers  PT_PHDR PHDRS; +  ph_text PT_LOAD FILEHDR PHDRS  FLAGS (0x1 | 0x4); +} +SECTIONS { + . = 0xffffffff20000000; + .text : { *(.text*) } : ph_text + .test 0x1000 : { BYTE(0) } + .bss : { *(.bss*) } +} + +## Section .test has VA 0x1000 and placed in the same segment as section .text +## with VA 0xffffffff20000000. That might be technically correct, but most probably +## is a result of a broken script file and causes file offset calculation overflow. +## It seems we do not have to support it, so we don't and we report an error in this case. +# ERR: error: unable to place section .text at file offset [0xFFFFFFFF20000000 -> 0xFFFFFFFE40000000]; check your linker script for overflows +# ERR-NOT: unable to place section .bss Index: test/ELF/linkerscript/Inputs/sections-va-overflow.s ===================================================================
>   - test/ELF/linkerscript/Inputs/sections-va-overflow.s +++ test/ELF/linkerscript/Inputs/sections-va-overflow.s @@ -0,0 +1,6 @@ +.global _start +_start: +  retq + +.bss +.space 0x2000 Index: ELF/Writer.cpp ===================================================================
>   - ELF/Writer.cpp +++ ELF/Writer.cpp @@ -1826,6 +1826,12 @@ } }
> 
>     +static std::string rangeToString(uint64_t Addr, uint64_t Len) { +  if (Len == 0) +    return "<empty range at 0x" + utohexstr(Addr) + ">"; +  return "[0x" + utohexstr(Addr) + " -> 0x" + utohexstr(Addr + Len - 1) + "]"; +} + // Adjusts the file alignment for a given output section and returns // its new file offset. The file offset must be the same with its // virtual address (modulo the page size) so that the loader can load @@ -1896,6 +1902,18 @@
> 
>     SectionHeaderOff = alignTo(Off, Config->Wordsize); FileSize = SectionHeaderOff + (OutputSections.size() + 1) * sizeof(Elf_Shdr); + +  // It is possible to get file offset overlaps or overflows with linker +  // scripts. We perform checks required in checkNoOverlappingSections() and +  // want to prevent file size overflows here because it would crash the linker. +  for (OutputSection *Sec : OutputSections) { +    if (Sec->Type == SHT_NOBITS) +      continue; +    if ((Sec->Offset > FileSize) || (Sec->Offset + Sec->Size > FileSize)) +      error("unable to place section " + Sec->Name + " at file offset " + +            rangeToString(Sec->Offset, Sec->Offset + Sec->Size) + +            "; check your linker script for overflows"); +  } }
> 
>     // Finalize the program headers. We call this function after we assign @@ -1934,12 +1952,6 @@ } }
> 
>     -static std::string rangeToString(uint64_t Addr, uint64_t Len) {
> - if (Len == 0)
> - return "<empty range at 0x" + utohexstr(Addr) + ">";
> - return "[0x" + utohexstr(Addr) + " -> 0x" + utohexstr(Addr + Len - 1) + "]"; -} - // Check whether sections overlap for a specific address range (file offsets, // load and virtual adresses). // _______________________________________________ llvm-commits mailing list llvm-commits at lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


https://reviews.llvm.org/D43819