ld/pdb: Fix PDB source-info substream overflow
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
source_files_count counts every source file reference across all
modules, not the deduplicated count that goes into the substream
header, so a uint16_t is too narrow. A link with more than 65535
references wraps it, allocates too small a buffer for the source-info
substream, and then writes past the end of it.
ld/
* pdb.c (create_source_info_substream): Make source_files_count a
uint32_t.
* testsuite/ld-pe/pdb4a.s: New test source.
* testsuite/ld-pe/pdb4b.s: New test source.
* testsuite/ld-pe/pdb.exp (test11): Run the new test.
---
ld/pdb.c | 2 +-
ld/testsuite/ld-pe/pdb.exp | 23 +++++++++++++++++++
ld/testsuite/ld-pe/pdb4a.s | 45 ++++++++++++++++++++++++++++++++++++++
ld/testsuite/ld-pe/pdb4b.s | 38 ++++++++++++++++++++++++++++++++
4 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-pe/pdb4a.s
create mode 100644 ld/testsuite/ld-pe/pdb4b.s
Comments
On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> --- a/ld/pdb.c
> +++ b/ld/pdb.c
> @@ -4359,7 +4359,7 @@ create_source_info_substream (void **data, uint32_t *size,
> struct source_files_info *source)
> {
> uint16_t dedupe_source_files_count = 0;
> - uint16_t source_files_count = 0;
> + uint32_t source_files_count = 0;
> uint32_t strings_len = 0;
> uint8_t *ptr;
Ah, here's the other half of the patch in PR 32942. The change is perhaps
okay as is, yet I'd still like to ask: Why would we need to use a fixed-
width type here in the first place? If this was properly "unsigned int",
then we would have been fine from the start (taking the assumption that
taken elsewhere as well: unsigned int being at least 32 bits wide; else
it should be unsigned long).
> --- /dev/null
> +++ b/ld/testsuite/ld-pe/pdb4a.s
> @@ -0,0 +1,45 @@
> +/* Two modules with 32768 source file references each, so that the total
> + number of references in the DBI source-info substream exceeds 65535. */
> +
> +.equ CV_SIGNATURE_C13, 4
> +.equ DEBUG_S_STRINGTABLE, 0xf3
> +.equ DEBUG_S_FILECHKSMS, 0xf4
> +.equ CHKSUM_TYPE_NONE, 0
> +
> +.equ NUM_CHKSMS, 32768
> +
> +.section ".debug$S", "rn"
> +.long CV_SIGNATURE_C13
> +.long DEBUG_S_STRINGTABLE
> +.long .strings_end - .strings_start
> +
> +.strings_start:
> +
> +.asciz ""
> +
> +.src1:
> +.asciz "foo"
> +
> +.strings_end:
> +
> +.balign 4
> +
> +.long DEBUG_S_FILECHKSMS
> +.long .chksms_end - .chksms_start
> +
> +.chksms_start:
> +
> +.rept NUM_CHKSMS
> +.long .src1 - .strings_start
> +.byte 0 /* checksum length */
> +.byte CHKSUM_TYPE_NONE
> +.short 0 /* padding */
> +.endr
> +
> +.chksms_end:
> +
> +.text
> +
> +.global main
> +main:
> + .long 0x12345678
Nit: At least in entirely new tests, please indent directive by at least a
blank (to distinguish them from labels).
Jan
@@ -4359,7 +4359,7 @@ create_source_info_substream (void **data, uint32_t *size,
struct source_files_info *source)
{
uint16_t dedupe_source_files_count = 0;
- uint16_t source_files_count = 0;
+ uint32_t source_files_count = 0;
uint32_t strings_len = 0;
uint8_t *ptr;
@@ -1837,6 +1837,28 @@ proc test10 { } {
}
}
+# Check that a PDB file with more than 65535 source file references can be
+# created.
+
+proc test11 { } {
+ global as
+ global ld
+ global srcdir
+ global subdir
+
+ if { ![ld_assemble $as $srcdir/$subdir/pdb4a.s tmpdir/pdb4a.o]
+ || ![ld_assemble $as $srcdir/$subdir/pdb4b.s tmpdir/pdb4b.o] } {
+ unsupported "Build pdb4a.o and pdb4b.o"
+ return
+ }
+
+ if ![ld_link $ld "tmpdir/pdb4.exe" "--pdb=tmpdir/pdb4.pdb -e main tmpdir/pdb4a.o tmpdir/pdb4b.o"] {
+ fail "Create PDB file with more than 65535 source file references"
+ } else {
+ pass "Create PDB file with more than 65535 source file references"
+ }
+}
+
test1
test2
test3
@@ -1847,3 +1869,4 @@ test7
test8
test9
test10
+test11
new file mode 100644
@@ -0,0 +1,45 @@
+/* Two modules with 32768 source file references each, so that the total
+ number of references in the DBI source-info substream exceeds 65535. */
+
+.equ CV_SIGNATURE_C13, 4
+.equ DEBUG_S_STRINGTABLE, 0xf3
+.equ DEBUG_S_FILECHKSMS, 0xf4
+.equ CHKSUM_TYPE_NONE, 0
+
+.equ NUM_CHKSMS, 32768
+
+.section ".debug$S", "rn"
+.long CV_SIGNATURE_C13
+.long DEBUG_S_STRINGTABLE
+.long .strings_end - .strings_start
+
+.strings_start:
+
+.asciz ""
+
+.src1:
+.asciz "foo"
+
+.strings_end:
+
+.balign 4
+
+.long DEBUG_S_FILECHKSMS
+.long .chksms_end - .chksms_start
+
+.chksms_start:
+
+.rept NUM_CHKSMS
+.long .src1 - .strings_start
+.byte 0 /* checksum length */
+.byte CHKSUM_TYPE_NONE
+.short 0 /* padding */
+.endr
+
+.chksms_end:
+
+.text
+
+.global main
+main:
+ .long 0x12345678
new file mode 100644
@@ -0,0 +1,38 @@
+/* See pdb4a.s. */
+
+.equ CV_SIGNATURE_C13, 4
+.equ DEBUG_S_STRINGTABLE, 0xf3
+.equ DEBUG_S_FILECHKSMS, 0xf4
+.equ CHKSUM_TYPE_NONE, 0
+
+.equ NUM_CHKSMS, 32768
+
+.section ".debug$S", "rn"
+.long CV_SIGNATURE_C13
+.long DEBUG_S_STRINGTABLE
+.long .strings_end - .strings_start
+
+.strings_start:
+
+.asciz ""
+
+.src1:
+.asciz "bar"
+
+.strings_end:
+
+.balign 4
+
+.long DEBUG_S_FILECHKSMS
+.long .chksms_end - .chksms_start
+
+.chksms_start:
+
+.rept NUM_CHKSMS
+.long .src1 - .strings_start
+.byte 0 /* checksum length */
+.byte CHKSUM_TYPE_NONE
+.short 0 /* padding */
+.endr
+
+.chksms_end: