Bug 28303 - objdump crashes in riscv_elf_add_sub_reloc
Summary: objdump crashes in riscv_elf_add_sub_reloc
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: ---
Assignee: Nick Clifton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-03 14:46 UTC by Shaohua Li
Modified: 2021-09-07 08:45 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2021-09-06 00:00:00


Attachments
poc (550 bytes, application/x-object)
2021-09-03 14:46 UTC, Shaohua Li
Details
Proposed patch (483 bytes, patch)
2021-09-06 11:09 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shaohua Li 2021-09-03 14:46:47 UTC
Created attachment 13647 [details]
poc

Hi there,

I crashed `objdump -S` with a fuzzer.

- Compiler: clang12
- Platform: Ubuntu 18.04.5 LTS, x86_64
- Reproduce: run `objdump -S poc`

AddressSanitizer report:

==2784==ERROR: AddressSanitizer: SEGV on unknown address 0x60e00001003f (pc 0x00000113c315 bp 0x7ffe695a69f0 sp 0x7ffe695a6700 T0)
==2784==The signal is caused by a READ memory access.
    #0 0x113c315 in riscv_elf_add_sub_reloc /binutils_latest/repo/bfd/elfxx-riscv.c:1005:23
    #1 0x1b4bae7 in bfd_perform_relocation /binutils_latest/repo/bfd/reloc.c:711:14
    #2 0x1b4fa9f in bfd_generic_get_relocated_section_contents /binutils_latest/repo/bfd/reloc.c:8463:10
    #3 0xae85cf in bfd_get_relocated_section_contents /binutils_latest/repo/bfd/bfd.c:2166:10
    #4 0xb11ef8 in bfd_simple_get_relocated_section_contents /binutils_latest/repo/bfd/simple.c:298:14
    #5 0xca6ba7 in _bfd_dwarf1_find_nearest_line /binutils_latest/repo/bfd/dwarf1.c:523:4
    #6 0xbd3447 in _bfd_elf_find_nearest_line /binutils_latest/repo/bfd/elf.c:9199:7
    #7 0x4e49ea in show_line /binutils_latest/repo/binutils/./objdump.c:1784:9
    #8 0x4e0b9f in disassemble_bytes /binutils_latest/repo/binutils/./objdump.c:2770:6
    #9 0x4daf10 in disassemble_section /binutils_latest/repo/binutils/./objdump.c:3455:4
    #10 0xb100da in bfd_map_over_sections /binutils_latest/repo/bfd/section.c:1383:5
    #11 0x4d1ae0 in disassemble_data /binutils_latest/repo/binutils/./objdump.c:3599:3
    #12 0x4cda84 in dump_bfd /binutils_latest/repo/binutils/./objdump.c:5006:5
    #13 0x4ccb9f in display_object_bfd /binutils_latest/repo/binutils/./objdump.c:5068:7
    #14 0x4ccaa9 in display_any_bfd /binutils_latest/repo/binutils/./objdump.c:5158:5
    #15 0x4cc65c in display_file /binutils_latest/repo/binutils/./objdump.c:5179:3
    #16 0x4cb063 in main /binutils_latest/repo/binutils/./objdump.c:5529:6
    #17 0x7f26086320b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #18 0x41c61d in _start (/out_bin/objdump+0x41c61d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /binutils_latest/repo/bfd/elfxx-riscv.c:1005:23 in riscv_elf_add_sub_reloc
==2784==ABORTING
Comment 1 Nick Clifton 2021-09-06 09:57:41 UTC
Investigating
Comment 2 Nick Clifton 2021-09-06 11:09:30 UTC
Created attachment 13655 [details]
Proposed patch

Possible patch to fix the problem
Comment 3 Palmer Dabbelt 2021-09-06 15:40:30 UTC
I see a bunch of

  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
    return bfd_reloc_outofrange;

in the MIPS port, which is subtly different than this.  I don't really care which error value to pick, but I guess ">" is a touch better than ">=".

Like Nick said, the proposed solution is sort of just a hack.  Given that it seems like a common idiom I'm fine with it, but it does seem preferable to hoist this into some generic code (I'd be tempted to try and check the bounds there, but IDK if that's sane).
Comment 4 Alan Modra 2021-09-07 00:46:06 UTC
The canonical check is

  octets = reloc_entry->address * bfd_octets_per_byte (abfd, input_section);
  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
                                 input_section, octets))
    return bfd_reloc_outofrange;

For riscv you can leave out bfd_octets_per_byte, or use an OCTETS_PER_BYTE macro defined to 1 as I do for ppc.
Comment 5 Sourceware Commits 2021-09-07 08:44:51 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3f1a2892e1fea343880b276474cb44db3abcaa9a

commit 3f1a2892e1fea343880b276474cb44db3abcaa9a
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Sep 7 09:44:17 2021 +0100

    Fix illegal memory access triggered by an attempt to disassemble a corrupt RISC-V binary.
    
            PR 28303
            * elfxx-riscv.c (riscv_elf_add_sub_reloc): Add check for out of
            range relocs.
Comment 6 Nick Clifton 2021-09-07 08:45:29 UTC
Thanks Alan.  I have gone ahead and applied a patch based upon your suggestion.