[v2] PE/COFF: raise normal PE section limit safely

Message ID 20260701175852.1111-2-oleg.tolmatcev@gmail.com
State New
Headers
Series [v2] PE/COFF: raise normal PE section limit safely |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm 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

Oleg Tolmatcev July 1, 2026, 5:58 p.m. UTC
  PE/COFF stores symbol section numbers in a 16-bit field.  Binutils used
signed 16-bit handling there, which limited normal PE objects to 32767
sections even though MSVC and Clang already accept a larger unsigned
range.

Raise the normal PE section limit to 65279, while keeping the PE/COFF
special section-number values for undefined, absolute and debug symbols
working correctly.  Do this by decoding and encoding normal PE symbol
section numbers as unsigned values in the ordinary range, while still
mapping the named reserved constants explicitly.

Also reject the remaining reserved PE symbol section-number values as
malformed input when reading symbols, instead of only stumbling over
them later in writer-side assertions.

Add a gas test that exercises a normal PE object above the old
32767-section limit and checks that objdump reports the high section
number correctly, and add a binutils test that corrupts a PE symbol
section number to verify that invalid reserved values are rejected.

bfd/ChangeLog:

        * coffcode.h (COFF_DEFAULT_MAX_NSCNS): Define.
        (bfd_coff_std_swap_table): Use it for the default maximum section
        count.
        (ticoff0_swap_table): Likewise.
        (ticoff1_swap_table): Likewise.
        (fill_comdat_hash): Fail on invalid PE symbol section numbers.
        (coff_set_arch_mach_hook): Likewise.
        * coffgen.c (coff_get_normalized_symtab): Likewise.
        * cofflink.c (coff_link_add_symbols): Likewise.
        (_bfd_coff_link_input_bfd): Likewise.
        * peXXigen.c (pe_decode_sym_section_number): New function.
        (pe_encode_sym_section_number): New function.
        (_bfd_XXi_swap_sym_in): Use pe_decode_sym_section_number.
        (_bfd_XXi_swap_sym_out): Use pe_encode_sym_section_number.

include/ChangeLog:

        * coff/pe.h (IMAGE_SYM_UNDEFINED): Define.
        (IMAGE_SYM_ABSOLUTE): Define.
        (IMAGE_SYM_DEBUG): Define.
        (IMAGE_SYM_SECTION_MAX): Define.

binutils/ChangeLog:

        * testsuite/binutils-all/objcopy.exp
        (objcopy_test_invalid_pe_symbol_section_number): New test.

gas/ChangeLog:

        * testsuite/gas/pe/pe.exp: Run large-obj-normal.
        * testsuite/gas/pe/large-obj-normal.s: New test.
        * testsuite/gas/pe/large-obj-normal.d: New test.

Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>
---
 bfd/coffcode.h                              | 19 ++++++-
 bfd/coffgen.c                               |  2 +
 bfd/cofflink.c                              |  8 +++
 bfd/peXXigen.c                              | 63 ++++++++++++++++++++-
 binutils/testsuite/binutils-all/objcopy.exp | 58 +++++++++++++++++++
 gas/testsuite/gas/pe/large-obj-normal.d     | 17 ++++++
 gas/testsuite/gas/pe/large-obj-normal.s     | 16 ++++++
 gas/testsuite/gas/pe/pe.exp                 |  1 +
 include/coff/pe.h                           |  5 ++
 9 files changed, 184 insertions(+), 5 deletions(-)
 create mode 100644 gas/testsuite/gas/pe/large-obj-normal.d
 create mode 100644 gas/testsuite/gas/pe/large-obj-normal.s
  

Comments

Jan Beulich July 3, 2026, 9:43 a.m. UTC | #1
On 01.07.2026 19:58, Oleg Tolmatcev wrote:
> --- a/bfd/peXXigen.c
> +++ b/bfd/peXXigen.c
> @@ -109,12 +109,70 @@
>  #define SetHighBit(val)      ((val) | 0x80000000)
>  #define WithoutHighBit(val)  ((val) & 0x7fffffff)
>  
> +static bool
> +pe_decode_sym_section_number (bfd *abfd, const char *raw_scnum, int *scnump)
> +{
> +  unsigned int scnum = H_GET_16 (abfd, raw_scnum);
> +
> +  switch (scnum)
> +    {
> +    case IMAGE_SYM_UNDEFINED:
> +      *scnump = N_UNDEF;
> +      return true;
> +    case IMAGE_SYM_ABSOLUTE:
> +      *scnump = N_ABS;
> +      return true;
> +    case IMAGE_SYM_DEBUG:
> +      *scnump = N_DEBUG;
> +      return true;
> +    }
> +
> +  if (scnum > IMAGE_SYM_SECTION_MAX)
> +    {
> +      _bfd_error_handler
> +	/* xgettext:c-format */
> +	(_("%pB: invalid PE symbol section number %#x"), abfd, scnum);

Mind me suggesting s/invalid/unrecognized/ ?

> +      bfd_set_error (bfd_error_bad_value);
> +      return false;
> +    }

While returning an error here may make sense, ...

> @@ -124,7 +182,8 @@ _bfd_XXi_swap_sym_in (bfd * abfd, void * ext1, void * in1)
>      memcpy (in->_n._n_name, ext->e.e_name, SYMNMLEN);
>  
>    in->n_value = H_GET_32 (abfd, ext->e_value);
> -  in->n_scnum = (short) H_GET_16 (abfd, ext->e_scnum);
> +  if (! pe_decode_sym_section_number (abfd, ext->e_scnum, &in->n_scnum))
> +    return;

... that's of limited use here when the caller can't be told of the error.
I don't think bfd_set_error() has any respective effect, unless that
caller actively checked via bfd_get_error(). IOW I think you still need to
set *scnump in pe_decode_sym_section_number(), and bailing out here isn't
helpful.

Oh, there is the ->read_only flag in struct bfd which looks to exist for
purposes like this one. It's for objcopy / strip only though, so in
particular ld could still be in trouble.

Jan
  

Patch

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 12cc7c3ca7..7353561407 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -376,6 +376,12 @@  extern const bfd_target TARGET_SYM_BIG;
 # define COFF_WITH_EXTENDED_RELOC_COUNTER
 #endif
 
+#if defined(COFF_WITH_PE) && !defined(COFF_IMAGE_WITH_PE)
+# define COFF_DEFAULT_MAX_NSCNS (IMAGE_SYM_SECTION_MAX + 1)
+#else
+# define COFF_DEFAULT_MAX_NSCNS 32768
+#endif
+
 #if defined (COFF_LONG_SECTION_NAMES)
 /* Needed to expand the inputs to BLANKOR1TOODD.  */
 #define COFFLONGSECTIONCATHELPER(x,y)    x ## y
@@ -927,6 +933,8 @@  fill_comdat_hash (bfd *abfd)
       flagword sec_flags = SEC_LINK_ONCE;
 
       bfd_coff_swap_sym_in (abfd, esym, &isym);
+      if (bfd_get_error () == bfd_error_bad_value)
+	return false;
 
       /* According to the MSVC documentation, the first TWO entries
 	 with the section # are both of interest to us.  The first one
@@ -2315,6 +2323,11 @@  coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 		if (buf == NULL)
 		  return false;
 		bfd_coff_swap_sym_in (abfd, buf, & sym);
+		if (bfd_get_error () == bfd_error_bad_value)
+		  {
+		    free (buf);
+		    return false;
+		  }
 		if (sym.n_sclass == C_FILE)
 		  cputype = sym.n_type & 0xff;
 		else
@@ -5615,7 +5628,7 @@  static const bfd_coff_backend_data bfd_coff_std_swap_table ATTRIBUTE_UNUSED =
 #else
   2,
 #endif
-  32768,
+  COFF_DEFAULT_MAX_NSCNS,
   coff_SWAP_filehdr_in, coff_SWAP_aouthdr_in, coff_SWAP_scnhdr_in,
   coff_SWAP_reloc_in, coff_bad_format_hook, coff_set_arch_mach_hook,
   coff_mkobject_hook, styp_to_sec_flags, coff_set_alignment_hook,
@@ -5656,7 +5669,7 @@  static const bfd_coff_backend_data ticoff0_swap_table =
 #else
   2,
 #endif
-  32768,
+  COFF_DEFAULT_MAX_NSCNS,
   coff_SWAP_filehdr_in, coff_SWAP_aouthdr_in, coff_SWAP_scnhdr_in,
   coff_swap_reloc_v0_in, ticoff0_bad_format_hook, coff_set_arch_mach_hook,
   coff_mkobject_hook, styp_to_sec_flags, coff_set_alignment_hook,
@@ -5698,7 +5711,7 @@  static const bfd_coff_backend_data ticoff1_swap_table =
 #else
   2,
 #endif
-  32768,
+  COFF_DEFAULT_MAX_NSCNS,
   coff_SWAP_filehdr_in, coff_SWAP_aouthdr_in, coff_SWAP_scnhdr_in,
   coff_SWAP_reloc_in, ticoff1_bad_format_hook, coff_set_arch_mach_hook,
   coff_mkobject_hook, styp_to_sec_flags, coff_set_alignment_hook,
diff --git a/bfd/coffgen.c b/bfd/coffgen.c
index f3060642a5..d2e2e905bc 100644
--- a/bfd/coffgen.c
+++ b/bfd/coffgen.c
@@ -1908,6 +1908,8 @@  coff_get_normalized_symtab (bfd *abfd)
 
       bfd_coff_swap_sym_in (abfd, (void *) raw_src,
 			    (void *) & internal_ptr->u.syment);
+      if (bfd_get_error () == bfd_error_bad_value)
+	return NULL;
       internal_ptr->is_sym = true;
       combined_entry_type *sym = internal_ptr;
 
diff --git a/bfd/cofflink.c b/bfd/cofflink.c
index e5c8a987d6..2de58c57bb 100644
--- a/bfd/cofflink.c
+++ b/bfd/cofflink.c
@@ -315,6 +315,8 @@  coff_link_add_symbols (bfd *abfd,
       bool copy;
 
       bfd_coff_swap_sym_in (abfd, esym, &sym);
+      if (bfd_get_error () == bfd_error_bad_value)
+	goto error_return;
 
       classification = bfd_coff_classify_symbol (abfd, &sym);
       if (classification != COFF_SYMBOL_LOCAL)
@@ -1485,6 +1487,8 @@  _bfd_coff_link_input_bfd (struct coff_final_link_info *flaginfo, bfd *input_bfd)
       int add;
 
       bfd_coff_swap_sym_in (input_bfd, esym, isymp);
+      if (bfd_get_error () == bfd_error_bad_value)
+	return false;
 
       /* Make a copy of *isymp so that the relocate_section function
 	 always sees the original values.  This is more reliable than
@@ -1684,6 +1688,8 @@  _bfd_coff_link_input_bfd (struct coff_final_link_info *flaginfo, bfd *input_bfd)
 	      char *name_copy;
 
 	      bfd_coff_swap_sym_in (input_bfd, esl, islp);
+	      if (bfd_get_error () == bfd_error_bad_value)
+		return false;
 
 	      amt = sizeof (struct coff_debug_merge_element);
 	      *epp = (struct coff_debug_merge_element *)
@@ -2278,6 +2284,8 @@  _bfd_coff_link_input_bfd (struct coff_final_link_info *flaginfo, bfd *input_bfd)
 					    (flaginfo->outsyms
 					     + ((indx - syment_base)
 						* osymesz)), &is);
+		      if (bfd_get_error () == bfd_error_bad_value)
+			return false;
 		      if ((ISFCN (is.n_type)
 			   || is.n_sclass == C_BLOCK)
 			  && is.n_numaux >= 1)
diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index bdb23dcbca..b5fb2a31a0 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -109,12 +109,70 @@ 
 #define SetHighBit(val)      ((val) | 0x80000000)
 #define WithoutHighBit(val)  ((val) & 0x7fffffff)
 
+static bool
+pe_decode_sym_section_number (bfd *abfd, const char *raw_scnum, int *scnump)
+{
+  unsigned int scnum = H_GET_16 (abfd, raw_scnum);
+
+  switch (scnum)
+    {
+    case IMAGE_SYM_UNDEFINED:
+      *scnump = N_UNDEF;
+      return true;
+    case IMAGE_SYM_ABSOLUTE:
+      *scnump = N_ABS;
+      return true;
+    case IMAGE_SYM_DEBUG:
+      *scnump = N_DEBUG;
+      return true;
+    }
+
+  if (scnum > IMAGE_SYM_SECTION_MAX)
+    {
+      _bfd_error_handler
+	/* xgettext:c-format */
+	(_("%pB: invalid PE symbol section number %#x"), abfd, scnum);
+      bfd_set_error (bfd_error_bad_value);
+      return false;
+    }
+
+  *scnump = scnum;
+  return true;
+}
+
+static void
+pe_encode_sym_section_number (bfd *abfd, int scnum, char *raw_scnum)
+{
+  unsigned int encoded_scnum;
+
+  switch (scnum)
+    {
+    case N_UNDEF:
+      encoded_scnum = IMAGE_SYM_UNDEFINED;
+      break;
+    case N_ABS:
+      encoded_scnum = IMAGE_SYM_ABSOLUTE;
+      break;
+    case N_DEBUG:
+      encoded_scnum = IMAGE_SYM_DEBUG;
+      break;
+    default:
+      BFD_ASSERT (scnum > 0 && (unsigned int) scnum <= IMAGE_SYM_SECTION_MAX);
+      encoded_scnum = scnum;
+      break;
+    }
+
+  H_PUT_16 (abfd, encoded_scnum, raw_scnum);
+}
+
 void
 _bfd_XXi_swap_sym_in (bfd * abfd, void * ext1, void * in1)
 {
   SYMENT *ext = (SYMENT *) ext1;
   struct internal_syment *in = (struct internal_syment *) in1;
 
+  memset (in, 0, sizeof (*in));
+
   if (ext->e.e_name[0] == 0)
     {
       in->_n._n_n._n_zeroes = 0;
@@ -124,7 +182,8 @@  _bfd_XXi_swap_sym_in (bfd * abfd, void * ext1, void * in1)
     memcpy (in->_n._n_name, ext->e.e_name, SYMNMLEN);
 
   in->n_value = H_GET_32 (abfd, ext->e_value);
-  in->n_scnum = (short) H_GET_16 (abfd, ext->e_scnum);
+  if (! pe_decode_sym_section_number (abfd, ext->e_scnum, &in->n_scnum))
+    return;
 
   if (sizeof (ext->e_type) == 2)
     in->n_type = H_GET_16 (abfd, ext->e_type);
@@ -262,7 +321,7 @@  _bfd_XXi_swap_sym_out (bfd * abfd, void * inp, void * extp)
     }
 
   H_PUT_32 (abfd, in->n_value, ext->e_value);
-  H_PUT_16 (abfd, in->n_scnum, ext->e_scnum);
+  pe_encode_sym_section_number (abfd, in->n_scnum, ext->e_scnum);
 
   if (sizeof (ext->e_type) == 2)
     H_PUT_16 (abfd, in->n_type, ext->e_type);
diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index 25e2a44096..ee5cf33b33 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -1293,6 +1293,62 @@  proc objcopy_test_elf_common_symbols {} {
     }
 }
 
+proc objcopy_test_invalid_pe_symbol_section_number {} {
+    global OBJDUMP
+    global OBJDUMPFLAGS
+    global srcdir
+    global subdir
+    global binutils_run_status
+
+    set test "objdump rejects invalid PE symbol section number"
+
+    if { ![is_pecoff_format] } {
+	return
+    }
+
+    if { [is_remote host] } {
+	untested $test
+	return
+    }
+
+    set objfile tmpdir/invalid-pe-scnum.o
+    if { ![binutils_assemble $srcdir/$subdir/bintest.s $objfile] } then {
+	unsupported $test
+	return
+    }
+
+    set file [open $objfile r+b]
+    fconfigure $file -translation binary
+    set data [read $file]
+
+    binary scan [string range $data 8 11] cu4 bytes
+    set symptr [expr {([lindex $bytes 0] << 0)
+		      | ([lindex $bytes 1] << 8)
+		      | ([lindex $bytes 2] << 16)
+		      | ([lindex $bytes 3] << 24)}]
+    set scnum_off [expr {$symptr + 12}]
+
+    set data [string replace $data $scnum_off [expr {$scnum_off + 1}] "\xFD\xFF"]
+
+    seek $file 0 start
+    puts -nonewline $file $data
+    close $file
+
+    set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS -t $objfile"]
+    if { $binutils_run_status == 0 } {
+	fail $test
+	return
+    }
+
+    if { ![regexp "invalid PE symbol section number 0xfffd" $got] } {
+	send_log "$got\n"
+	fail $test
+	return
+    }
+
+    pass $test
+}
+
 # ia64 specific tests
 if { ([istarget "ia64-*-elf*"]
        || [istarget "ia64-*-linux*"]) } {
@@ -1581,6 +1637,8 @@  if { ![is_xcoff_format] } {
 
 run_dump_test "rename-section-01"
 
+objcopy_test_invalid_pe_symbol_section_number
+
 proc objcopy_tek2bin {} {
     global OBJCOPY
     global OBJDUMP
diff --git a/gas/testsuite/gas/pe/large-obj-normal.d b/gas/testsuite/gas/pe/large-obj-normal.d
new file mode 100644
index 0000000000..e9009e5795
--- /dev/null
+++ b/gas/testsuite/gas/pe/large-obj-normal.d
@@ -0,0 +1,17 @@ 
+#objdump: -h -t
+#name: PE large normal object after raising the normal section limit
+
+# This only becomes meaningful once normal PE/COFF is allowed past the
+# old 32767-section cutoff.  Before that, BFD errors out or auto-promotes
+# to bigobj before the normal 16-bit symbol-section encoding is exercised.
+
+.*: *file format pe-(aarch64-little|i386|x86-64)
+
+Sections:
+#...
+40002 +\.data\$a39999  .*
+                  CONTENTS, ALLOC, LOAD, DATA
+
+SYMBOL TABLE:
+#...
+.*\(sec 40003\).*\(scl +2\).*\) 0x[0-9a-f]+ a39999$
diff --git a/gas/testsuite/gas/pe/large-obj-normal.s b/gas/testsuite/gas/pe/large-obj-normal.s
new file mode 100644
index 0000000000..254733023d
--- /dev/null
+++ b/gas/testsuite/gas/pe/large-obj-normal.s
@@ -0,0 +1,16 @@ 
+	.file "large-obj-normal.s"
+
+	.irp n,0,1,2,3
+	.irp m,0,1,2,3,4,5,6,7,8,9
+	.irp c,0,1,2,3,4,5,6,7,8,9
+	.irp d,0,1,2,3,4,5,6,7,8,9
+	.irp u,0,1,2,3,4,5,6,7,8,9
+	.globl a\n\m\c\d\u
+	.section .data$a\n\m\c\d\u,"w"
+a\n\m\c\d\u :
+	.byte 1
+	.endr
+	.endr
+	.endr
+	.endr
+	.endr
diff --git a/gas/testsuite/gas/pe/pe.exp b/gas/testsuite/gas/pe/pe.exp
index 82f217cf94..600fd2425a 100644
--- a/gas/testsuite/gas/pe/pe.exp
+++ b/gas/testsuite/gas/pe/pe.exp
@@ -71,6 +71,7 @@  if {[istarget "aarch64-*-pe*"] || [istarget "aarch64-*-mingw*"]} {
 if { [istarget "aarch64-*-*"]
      || ([istarget "i*86-*-*"] && ![istarget "*-*-interix*"])
      || [istarget "x86_64-*-*"] } then {
+	run_dump_test "large-obj-normal"
 	run_dump_test "big-obj"
 	run_dump_test "big-obj-auto"
 }
diff --git a/include/coff/pe.h b/include/coff/pe.h
index ece4bf25db..133548ff4b 100644
--- a/include/coff/pe.h
+++ b/include/coff/pe.h
@@ -388,6 +388,11 @@  struct external_SYMBOL_EX
   char e_numaux[1];
 } ATTRIBUTE_PACKED ;
 
+#define IMAGE_SYM_UNDEFINED      0
+#define IMAGE_SYM_ABSOLUTE       0xffff
+#define IMAGE_SYM_DEBUG          0xfffe
+#define IMAGE_SYM_SECTION_MAX    0xfeff
+
 #define	SYMENT_BIGOBJ	struct external_SYMBOL_EX
 #define	SYMESZ_BIGOBJ	20