[llvm] r366796 - [Object/ELF.h] - Improve testing of the fields in ELFFile<ELFT>::sections().

Vlad Tsyrklevich via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 23 08:01:48 PDT 2019


I've reverted this change as it was causing buildbot failures on the UBSan
bot like the following
<http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/14047/steps/check-llvm%20ubsan/logs/stdio>
:

/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/Object/invalid.test:600:21:
error: INVALID-SEC-NUM3: expected string not found in input
# INVALID-SEC-NUM3: error: '[[FILE]]': invalid section header table
offset (e_shoff = 0xffffffffffffffff) or invalid number of sections
specified in the first section header's sh_size field (0x1)
                    ^
<stdin>:1:1: note: scanning from here
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/include/llvm/Object/ELF.h:509:49:
runtime error: addition of unsigned offset to 0x000002106cf0
overflowed to 0x000002106cef
^
<stdin>:1:1: note: with "FILE" equal to
"/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/Object/Output/invalid\\.test\\.tmp28"
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/include/llvm/Object/ELF.h:509:49:
runtime error: addition of unsigned offset to 0x000002106cf0
overflowed to 0x000002106cef


On Tue, Jul 23, 2019 at 4:36 AM George Rimar via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: grimar
> Date: Tue Jul 23 04:37:14 2019
> New Revision: 366796
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366796&view=rev
> Log:
> [Object/ELF.h] - Improve testing of the fields in
> ELFFile<ELFT>::sections().
>
> This eliminates a one error untested and
> also introduces a error for one more possible case
> which lead to crash previously.
>
> Differential revision: https://reviews.llvm.org/D64987
>
> Modified:
>     llvm/trunk/include/llvm/Object/ELF.h
>     llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h
>     llvm/trunk/test/Object/invalid.test
>     llvm/trunk/tools/yaml2obj/yaml2elf.cpp
>
> Modified: llvm/trunk/include/llvm/Object/ELF.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ELF.h?rev=366796&r1=366795&r2=366796&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Object/ELF.h (original)
> +++ llvm/trunk/include/llvm/Object/ELF.h Tue Jul 23 04:37:14 2019
> @@ -513,15 +513,22 @@ Expected<typename ELFT::ShdrRange> ELFFi
>      NumSections = First->sh_size;
>
>    if (NumSections > UINT64_MAX / sizeof(Elf_Shdr))
> -    // TODO: this error is untested.
> -    return createError("section table goes past the end of file");
> +    return createError("invalid number of sections specified in the NULL "
> +                       "section's sh_size field (" +
> +                       Twine(NumSections) + ")");
>
>    const uint64_t SectionTableSize = NumSections * sizeof(Elf_Shdr);
> +  if (SectionTableOffset + SectionTableSize < SectionTableOffset)
> +    return createError(
> +        "invalid section header table offset (e_shoff = 0x" +
> +        Twine::utohexstr(SectionTableOffset) +
> +        ") or invalid number of sections specified in the first section "
> +        "header's sh_size field (0x" +
> +        Twine::utohexstr(NumSections) + ")");
>
>    // Section table goes past end of file!
>    if (SectionTableOffset + SectionTableSize > FileSize)
>      return createError("section table goes past the end of file");
> -
>    return makeArrayRef(First, NumSections);
>  }
>
>
> Modified: llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h?rev=366796&r1=366795&r2=366796&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h (original)
> +++ llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h Tue Jul 23 04:37:14 2019
> @@ -77,7 +77,7 @@ struct FileHeader {
>    llvm::yaml::Hex64 Entry;
>
>    Optional<llvm::yaml::Hex16> SHEntSize;
> -  Optional<llvm::yaml::Hex16> SHOffset;
> +  Optional<llvm::yaml::Hex64> SHOffset;
>    Optional<llvm::yaml::Hex16> SHNum;
>    Optional<llvm::yaml::Hex16> SHStrNdx;
>  };
>
> Modified: llvm/trunk/test/Object/invalid.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/invalid.test?rev=366796&r1=366795&r2=366796&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Object/invalid.test (original)
> +++ llvm/trunk/test/Object/invalid.test Tue Jul 23 04:37:14 2019
> @@ -552,3 +552,58 @@ FileHeader:
>  Sections:
>    - Name: .foo
>      Type: SHT_PROGBITS
> +
> +## We report an error if the number of sections stored in sh_size
> +## is greater than UINT64_MAX / sizeof(Elf_Shdr) == 288230376151711743.
> +## Here we check that do not crash on a border value.
> +
> +# RUN: yaml2obj --docnum=26 %s -o %t26
> +# RUN: not llvm-readobj -h %t26 2>&1 | FileCheck -DFILE=%t26
> --check-prefix=INVALID-SEC-NUM1 %s
> +
> +# INVALID-SEC-NUM1: error: '[[FILE]]': invalid section header table
> offset (e_shoff = 0x40) or invalid number of sections specified in the
> first section header's sh_size field (0x3ffffffffffffff)
> +
> +--- !ELF
> +FileHeader:
> +  Class:   ELFCLASS64
> +  Data:    ELFDATA2LSB
> +  Type:    ET_REL
> +  Machine: EM_X86_64
> +  SHNum:   0x0
> +Sections:
> +  - Type: SHT_NULL
> +    Size: 288230376151711743
> +
> +## See above, but now we test the UINT64_MAX / sizeof(Elf_Shdr) value.
> +## The error is slightly different in this case.
> +
> +# RUN: yaml2obj --docnum=27 %s -o %t27
> +# RUN: not llvm-readobj -h %t27 2>&1 | FileCheck -DFILE=%t27
> --check-prefix=INVALID-SEC-NUM2 %s
> +
> +# INVALID-SEC-NUM2: error: '[[FILE]]': invalid number of sections
> specified in the NULL section's sh_size field (288230376151711744)
> +
> +--- !ELF
> +FileHeader:
> +  Class:   ELFCLASS64
> +  Data:    ELFDATA2LSB
> +  Type:    ET_REL
> +  Machine: EM_X86_64
> +  SHNum:   0x0
> +Sections:
> +  - Type: SHT_NULL
> +    Size: 288230376151711744
> +
> +## Check the case when SHOffset is too large, but SHNum is not. SHOffset
> + SHNum overflows the uint64 type.
> +
> +# RUN: yaml2obj --docnum=28 %s -o %t28
> +# RUN: not llvm-readobj -h %t28 2>&1 | FileCheck -DFILE=%t28
> --check-prefix=INVALID-SEC-NUM3 %s
> +
> +# INVALID-SEC-NUM3: error: '[[FILE]]': invalid section header table
> offset (e_shoff = 0xffffffffffffffff) or invalid number of sections
> specified in the first section header's sh_size field (0x1)
> +
> +--- !ELF
> +FileHeader:
> +  Class:    ELFCLASS64
> +  Data:     ELFDATA2LSB
> +  Type:     ET_REL
> +  Machine:  EM_X86_64
> +  SHOffset: 0xffffffffffffffff
> +  SHNum:    0x1
>
> Modified: llvm/trunk/tools/yaml2obj/yaml2elf.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/yaml2obj/yaml2elf.cpp?rev=366796&r1=366795&r2=366796&view=diff
>
> ==============================================================================
> --- llvm/trunk/tools/yaml2obj/yaml2elf.cpp (original)
> +++ llvm/trunk/tools/yaml2obj/yaml2elf.cpp Tue Jul 23 04:37:14 2019
> @@ -244,7 +244,7 @@ void ELFState<ELFT>::initELFHeader(Elf_E
>    // Immediately following the ELF header and program headers.
>    Header.e_shoff =
>        Doc.Header.SHOffset
> -          ? (uint16_t)*Doc.Header.SHOffset
> +          ? (typename ELFT::uint)(*Doc.Header.SHOffset)
>            : sizeof(Header) + sizeof(Elf_Phdr) * Doc.ProgramHeaders.size();
>    Header.e_shnum =
>        Doc.Header.SHNum ? (uint16_t)*Doc.Header.SHNum : SN2I.size() + 1;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190723/868d86d3/attachment.html>


More information about the llvm-commits mailing list