Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libbacktrace: fix unaligned read of the header of the compressed debug section #120

Conversation

ubyte
Copy link

@ubyte ubyte commented Mar 7, 2024

The type b_elf_chd requires 8 byte alignment on a 64-bit platform, but a compressed debug section may be stored with less restrictive alignment.
That misalignment read may cause program termination if it was compiled with the UndefinedBehaviorSanitizer enabled.

@ianlancetaylor
Copy link
Owner

Have you seen a real program with a misaligned SHF_COMPRESSED section? I would expect that tools that generate such sections would use an sh_align field that keeps the compressed section properly aligned. That is what I see when testing with the GNU linker.

@ubyte
Copy link
Author

ubyte commented Mar 8, 2024

I'm using LD LLD linker from the LLVM project.

$ clang++ --version
clang version 16.0.6
Target: x86_64-unknown-linux-gnu

$ ./ld.lld  --version
LLD 16.0.6 (compatible with GNU linkers)

Have you seen a real program with a misaligned SHF_COMPRESSED section?

Yes, it was a real program for which I recently started using the -gz=zlib option to build.
In fact, almost every program produced by this toolchain began to have unaligned debug sections.
This didn't cause any problems until I tried to run the program with the UB-sanitizer.

This is how the output of the readelf --sections looks like for a simple program
$ readelf --sections  --wide ./a.out
There are 44 section headers, starting at offset 0x2884d0:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .interp           PROGBITS        00000000002002a8 0002a8 00001c 00   A  0   0  1
  [ 2] .note.ABI-tag     NOTE            00000000002002c4 0002c4 000020 00   A  0   0  4
  [ 3] .note.gnu.build-id NOTE            00000000002002e4 0002e4 000024 00   A  0   0  4
  [ 4] .dynsym           DYNSYM          0000000000200308 000308 002130 18   A  8   1  8
  [ 5] .gnu.version      VERSYM          0000000000202438 002438 0002c4 02   A  4   0  2
  [ 6] .gnu.version_r    VERNEED         00000000002026fc 0026fc 000080 00   A  8   3  4
  [ 7] .gnu.hash         GNU_HASH        0000000000202780 002780 000774 00   A  4   0  8
  [ 8] .dynstr           STRTAB          0000000000202ef4 002ef4 001a1e 00   A  0   0  1
  [ 9] .rela.dyn         RELA            0000000000204918 004918 000180 18   A  4   0  8
  [10] .rela.plt         RELA            0000000000204a98 004a98 0006a8 18  AI  4  29  8
  [11] .rodata           PROGBITS        0000000000205140 005140 0127dc 00 AMS  0   0 16
  [12] .gcc_except_table PROGBITS        000000000021791c 01791c 0003a4 00   A  0   0  4
  [13] .eh_frame_hdr     PROGBITS        0000000000217cc0 017cc0 001d44 00   A  0   0  4
  [14] .eh_frame         PROGBITS        0000000000219a08 019a08 009cf0 00   A  0   0  8
  [15] .text             PROGBITS        0000000000223700 023700 08b27a 00  AX  0   0 16
  [16] .init             PROGBITS        00000000002ae97c 0ae97c 00001a 00  AX  0   0  4
  [17] .fini             PROGBITS        00000000002ae998 0ae998 000009 00  AX  0   0  4
  [18] .plt              PROGBITS        00000000002ae9b0 0ae9b0 000480 00  AX  0   0 16
  [19] .tbss             NOBITS          00000000002aee30 0aee30 000008 00 WAT  0   0  8
  [20] .jcr              PROGBITS        00000000002afe30 0aee30 000008 00  WA  0   0  8
  [21] .fini_array       FINI_ARRAY      00000000002afe38 0aee38 000008 00  WA  0   0  8
  [22] .init_array       INIT_ARRAY      00000000002afe40 0aee40 000020 00  WA  0   0  8
  [23] .data.rel.ro      PROGBITS        00000000002afe60 0aee60 0033d8 00  WA  0   0 16
  [24] .preinit_array    PREINIT_ARRAY   00000000002b3238 0b2238 000008 00  WA  0   0  8
  [25] .dynamic          DYNAMIC         00000000002b3240 0b2240 000200 10  WA  8   0  8
  [26] .got              PROGBITS        00000000002b3440 0b2440 0000b0 00  WA  0   0  8
  [27] .tm_clone_table   PROGBITS        00000000002b44f0 0b24f0 000000 00  WA  0   0  8
  [28] .data             PROGBITS        00000000002b44f0 0b24f0 025000 00  WA  0   0 16
  [29] .got.plt          PROGBITS        00000000002d94f0 0d74f0 000250 00  WA  0   0  8
  [30] .bss              NOBITS          00000000002d9740 0d7740 5a6fd0 00  WA  0   0 64
  [31] .gnu_debuglink    PROGBITS        0000000000000000 0d7740 000024 00      0   0  1
  [32] .comment          PROGBITS        0000000000000000 0d7764 000054 01  MS  0   0  1
  [33] .debug_loc        PROGBITS        0000000000000000 0d77b8 03bb49 00   C  0   0  1
  [34] .debug_abbrev     PROGBITS        0000000000000000 113301 007299 00   C  0   0  1
  [35] .debug_info       PROGBITS        0000000000000000 11a59a 07ca20 00   C  0   0  1
  [36] .debug_str        PROGBITS        0000000000000000 196fba 0254f9 01 MSC  0   0  1
  [37] .debug_line       PROGBITS        0000000000000000 1bc4b3 030ac5 00   C  0   0  1
  [38] .debug_ranges     PROGBITS        0000000000000000 1ecf78 00bd1e 00   C  0   0  1
  [39] .debug_aranges    PROGBITS        0000000000000000 1f8c96 000047 00   C  0   0  1
  [40] .gdb_index        PROGBITS        0000000000000000 1f8cdd 06c2f0 00      0   0  1
  [41] .symtab           SYMTAB          0000000000000000 264fd0 00ded8 18     43 1894  8
  [42] .shstrtab         STRTAB          0000000000000000 272ea8 0001c4 00      0   0  1
  [43] .strtab           STRTAB          0000000000000000 27306c 01545e 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  D (mbind), l (large), p (processor specific)

@ianlancetaylor
Copy link
Owner

Thanks. That looks like a bug in lld. It should be setting sh_align to 8, not 1, for a compressed section.

I guess we should work around the bug, though.

ianlancetaylor added a commit that referenced this pull request Mar 8, 2024
Patch originally by GitHub user ubyte at
#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
ianlancetaylor added a commit to ianlancetaylor/cgosymbolizer that referenced this pull request Mar 8, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
@ianlancetaylor
Copy link
Owner

Thanks, I committed the patch upstream and merged it back into this repo.

hubot pushed a commit to gcc-mirror/gcc that referenced this pull request Mar 8, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
@ubyte
Copy link
Author

ubyte commented Mar 8, 2024

Thank you!

@ubyte ubyte deleted the fix-unaligned-read-compressed-section branch March 8, 2024 22:22
@MaskRay
Copy link

MaskRay commented Mar 8, 2024

On ELF64, it looks like BFD uses 8-byte alignment for compressed .debug_* sections while gold/lld/mold use 1-byte alignment. I do not know how the Solaris linker sets the alignment.

The specification's wording makes me confused whether it really requires 8-byte alignment, even if a non-packed Elf64_Chdr surely requires 8.

The sh_size and sh_addralign fields of the section header for a compressed section reflect the requirements of the compressed section.

There are many .debug_* sections. So avoiding some alignment padding seems a very natural extension, even if the specification doesn't allow it with a very strict interpretation.

@ianlancetaylor
Copy link
Owner

The compressed section starts with a struct that requires 8-byte alignment on a 64-bit system, so it seems to me that the compressed section should have 8-byte alignment. That's how every other ELF structure works, after all. If this is an exception, then the standard should explicitly call it out as such. What it does say is that sh_align is the alignment requirement for the compressed section, which to me means the alignment required for the data in that section, which to me means requiring 8-byte alignment.

Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
Liaoshihua pushed a commit to Liaoshihua/ruyi-gcc that referenced this pull request Mar 11, 2024
Patch originally by GitHub user ubyte at
ianlancetaylor/libbacktrace#120.

	* elf.c (elf_uncompress_chdr): Don't assume compressed section is
	aligned.
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.

None yet

3 participants