[2/3] bfd/PE: fix bigobj file corruption on objcopy
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
Converting any linked executable to pe-bigobj-* silently produced
corrupted section headers. For example:
$ gcc -g -o testprog testprog.c
$ objcopy -I elf64-x86-64 -O pe-bigobj-x86-64 testprog testprog.bigobj
$ objdump -b pe-bigobj-x86-64 -h testprog.bigobj | head
[...]
Idx Name Size VMA LMA File off Algn
0 ^K^B^B/ 00001050 0000000000000000 0000000000000000 00001050 2**2
bigobj files can't have an OptionalHeader. coff_bigobj_swap_filehdr_in
therefore correctly hardcodes f_opthdr = 0 when reading bigobj into
memory.
However, bigobj flags wrongly allowed for EXEC_P which requests an
OptionalHeader to store a start address in multiple code paths. Further,
bigobj's _bfd_aoutsz was initialized > 0 and therefore miscalculated the
section header offset on write as if there was an OptionalHeader,
causing an invalid gap. If a correct reader reads the produced image, it
tries to read section headers but actually accesses undefined content
from the gap.
Fix it by ensuring bigobj formats have _bfd_aoutsz = 0 and EXEC_P unset,
notably also for special cases like auto-promotion. This causes minor
information loss (flag, start address) on some conversions, but it's
inevitable if the format just can't represent it and still better than
producing invalid output.
Signed-off-by: Tobias Deiminger <tobias.deiminger@linutronix.de>
---
Here are two more ways to trigger the bug.
File corruption during ld
$ printf '.globl _entry\n.text\n_entry: ret\n' > entry.s
$ x86_64-w64-mingw32-as -o entry.o entry.s
$ ld -m i386pep --oformat pe-bigobj-x86-64 -e _entry -o linked.o entry.o
$ objdump -h linked.o | head
[...]
Idx Name Size VMA LMA File off Algn
0 ^K^B^B/ 00001000 0000000000000000 0000000000000000 00001000 2**2
File corruption during auto-promotion (more than 65279 sections)
$ printf '.section .t%d,"dr"\n.byte 1\n' $(seq 0 65999) > many.s
$ x86_64-w64-mingw32-as -mbig-obj -o many.o many.s
$ objcopy -I pe-bigobj-x86-64 -O pe-x86-64 --set-start 0x1000 many.o bigmany.o
$ objdump -h bigmany.o | head
[...]
Idx Name Size VMA LMA File off Algn
0 ^K^B^B/ 00001000 0000000000000000 0000000000000000 00000000 2**2
bfd/coff-aarch64.c | 6 +++---
bfd/coff-i386.c | 6 +++---
bfd/coff-x86_64.c | 6 +++---
bfd/coffcode.h | 23 ++++++++++++++++++-----
4 files changed, 27 insertions(+), 14 deletions(-)
Comments
On 28.09.2026 11:01, Tobias Deiminger wrote:
> Converting any linked executable to pe-bigobj-* silently produced
> corrupted section headers. For example:
>
> $ gcc -g -o testprog testprog.c
> $ objcopy -I elf64-x86-64 -O pe-bigobj-x86-64 testprog testprog.bigobj
> $ objdump -b pe-bigobj-x86-64 -h testprog.bigobj | head
> [...]
> Idx Name Size VMA LMA File off Algn
> 0 ^K^B^B/ 00001050 0000000000000000 0000000000000000 00001050 2**2
>
> bigobj files can't have an OptionalHeader. coff_bigobj_swap_filehdr_in
> therefore correctly hardcodes f_opthdr = 0 when reading bigobj into
> memory.
>
> However, bigobj flags wrongly allowed for EXEC_P which requests an
> OptionalHeader to store a start address in multiple code paths. Further,
> bigobj's _bfd_aoutsz was initialized > 0 and therefore miscalculated the
> section header offset on write as if there was an OptionalHeader,
> causing an invalid gap. If a correct reader reads the produced image, it
> tries to read section headers but actually accesses undefined content
> from the gap.
>
> Fix it by ensuring bigobj formats have _bfd_aoutsz = 0 and EXEC_P unset,
> notably also for special cases like auto-promotion. This causes minor
> information loss (flag, start address) on some conversions, but it's
> inevitable if the format just can't represent it and still better than
> producing invalid output.
While things read plausibly (assuming one is willing to accept that
converting a linked binary back into an object file is something that
wants permitting), I'm struggling with the solution. There shouldn't be
a difference between settings (EXEC_P, AOUTSZ) for bigobj vs "normal".
That difference is rather one between (COFF) objects and linked (PE)
binaries, isn't it?
That is, shouldn't e.g. pe-<arch>.c and pei-<arch>.c control
(differently) what coff-<arch>.c (aided by coffcode.h) puts in place?
(Really this may not need any change to pe{,i}-<arch>.c, but may be
possible to control via use of COFF_IMAGE_WITH_PE directly in
coff-<arch>.c / coffcode.h. And then uniformly for bigobj and "normal"
objects. Likely the !COFF_WITH_PE case also would need leaving alone
altogether, just to avoid breaking anything there.)
Jan
Am Freitag, 2. Oktober 2026, 14:46:43 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
> On 28.09.2026 11:01, Tobias Deiminger wrote:
> > Converting any linked executable to pe-bigobj-* silently produced
> > corrupted section headers. For example:
> >
> > $ gcc -g -o testprog testprog.c
> > $ objcopy -I elf64-x86-64 -O pe-bigobj-x86-64 testprog testprog.bigobj
> > $ objdump -b pe-bigobj-x86-64 -h testprog.bigobj | head
> > [...]
> > Idx Name Size VMA LMA File off Algn
> > 0 ^K^B^B/ 00001050 0000000000000000 0000000000000000 00001050 2**2
> >
> > bigobj files can't have an OptionalHeader. coff_bigobj_swap_filehdr_in
> > therefore correctly hardcodes f_opthdr = 0 when reading bigobj into
> > memory.
> >
> > However, bigobj flags wrongly allowed for EXEC_P which requests an
> > OptionalHeader to store a start address in multiple code paths. Further,
> > bigobj's _bfd_aoutsz was initialized > 0 and therefore miscalculated the
> > section header offset on write as if there was an OptionalHeader,
> > causing an invalid gap. If a correct reader reads the produced image, it
> > tries to read section headers but actually accesses undefined content
> > from the gap.
> >
> > Fix it by ensuring bigobj formats have _bfd_aoutsz = 0 and EXEC_P unset,
> > notably also for special cases like auto-promotion. This causes minor
> > information loss (flag, start address) on some conversions, but it's
> > inevitable if the format just can't represent it and still better than
> > producing invalid output.
>
> While things read plausibly (assuming one is willing to accept that
> converting a linked binary back into an object file is something that
> wants permitting), I'm struggling with the solution. There shouldn't be
> a difference between settings (EXEC_P, AOUTSZ) for bigobj vs "normal".
> That difference is rather one between (COFF) objects and linked (PE)
> binaries, isn't it?
In my understanding it's like this:
- objects: *should not have* an optional header, but could
- bigobj: definitely *cannot have* an optional header
- EXEC_P only makes sense with an optional header, i.e. autosz > 0.
COFF File Header (Object and Image) says [1]
SizeOfOptionalHeader ... This value should be zero for an object file.
The "should" leaves the odd possibility that an object could have a non-zero
optional header. objcopy can create such odd, yet valid objects today. Whether
it makes sense I don't know.
However, bigobj has a distinct file header
// bigobj doesn't use COFF File Header, but a special one
typedef struct ANON_OBJECT_HEADER_BIGOBJ {
...
// no SizeOfOptionalHeader field here at all
...
}
That means bigobj can definitely not have an optional header, because the
bigobj file header has no field to declare room for an optional header.
> That is, shouldn't e.g. pe-<arch>.c and pei-<arch>.c control
> (differently) what coff-<arch>.c (aided by coffcode.h) puts in place?
> (Really this may not need any change to pe{,i}-<arch>.c, but may be
> possible to control via use of COFF_IMAGE_WITH_PE directly in
> coff-<arch>.c / coffcode.h. And then uniformly for bigobj and "normal"
> objects. Likely the !COFF_WITH_PE case also would need leaving alone
> altogether, just to avoid breaking anything there.)
I wouldn't have been confident enough to change all PE formats in BFD so that
"should be zero" for us means "we don't support an optional header". Thus kept
the scope local to bigobj.
Do you still prefer a PE-wide change? If so I'll add it to v2.
Tobias
> Jan
>
On 06.10.2026 09:44, Tobias Deiminger wrote:
> Am Freitag, 2. Oktober 2026, 14:46:43 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
>> On 28.09.2026 11:01, Tobias Deiminger wrote:
>>> Converting any linked executable to pe-bigobj-* silently produced
>>> corrupted section headers. For example:
>>>
>>> $ gcc -g -o testprog testprog.c
>>> $ objcopy -I elf64-x86-64 -O pe-bigobj-x86-64 testprog testprog.bigobj
>>> $ objdump -b pe-bigobj-x86-64 -h testprog.bigobj | head
>>> [...]
>>> Idx Name Size VMA LMA File off Algn
>>> 0 ^K^B^B/ 00001050 0000000000000000 0000000000000000 00001050 2**2
>>>
>>> bigobj files can't have an OptionalHeader. coff_bigobj_swap_filehdr_in
>>> therefore correctly hardcodes f_opthdr = 0 when reading bigobj into
>>> memory.
>>>
>>> However, bigobj flags wrongly allowed for EXEC_P which requests an
>>> OptionalHeader to store a start address in multiple code paths. Further,
>>> bigobj's _bfd_aoutsz was initialized > 0 and therefore miscalculated the
>>> section header offset on write as if there was an OptionalHeader,
>>> causing an invalid gap. If a correct reader reads the produced image, it
>>> tries to read section headers but actually accesses undefined content
>>> from the gap.
>>>
>>> Fix it by ensuring bigobj formats have _bfd_aoutsz = 0 and EXEC_P unset,
>>> notably also for special cases like auto-promotion. This causes minor
>>> information loss (flag, start address) on some conversions, but it's
>>> inevitable if the format just can't represent it and still better than
>>> producing invalid output.
>>
>> While things read plausibly (assuming one is willing to accept that
>> converting a linked binary back into an object file is something that
>> wants permitting), I'm struggling with the solution. There shouldn't be
>> a difference between settings (EXEC_P, AOUTSZ) for bigobj vs "normal".
>> That difference is rather one between (COFF) objects and linked (PE)
>> binaries, isn't it?
>
> In my understanding it's like this:
> - objects: *should not have* an optional header, but could
> - bigobj: definitely *cannot have* an optional header
> - EXEC_P only makes sense with an optional header, i.e. autosz > 0.
>
> COFF File Header (Object and Image) says [1]
>
> SizeOfOptionalHeader ... This value should be zero for an object file.
>
> The "should" leaves the odd possibility that an object could have a non-zero
> optional header. objcopy can create such odd, yet valid objects today. Whether
> it makes sense I don't know.
Imo such uses of "should" often are an attempt to be kind, i.e. to avoid
words like "must" or "has to". Hence I question whether object files with
the field non-zero actually are valid.
> However, bigobj has a distinct file header
>
> // bigobj doesn't use COFF File Header, but a special one
> typedef struct ANON_OBJECT_HEADER_BIGOBJ {
> ...
> // no SizeOfOptionalHeader field here at all
> ...
> }
>
> That means bigobj can definitely not have an optional header, because the
> bigobj file header has no field to declare room for an optional header.
>
>> That is, shouldn't e.g. pe-<arch>.c and pei-<arch>.c control
>> (differently) what coff-<arch>.c (aided by coffcode.h) puts in place?
>> (Really this may not need any change to pe{,i}-<arch>.c, but may be
>> possible to control via use of COFF_IMAGE_WITH_PE directly in
>> coff-<arch>.c / coffcode.h. And then uniformly for bigobj and "normal"
>> objects. Likely the !COFF_WITH_PE case also would need leaving alone
>> altogether, just to avoid breaking anything there.)
>
> I wouldn't have been confident enough to change all PE formats in BFD so that
> "should be zero" for us means "we don't support an optional header". Thus kept
> the scope local to bigobj.
>
> Do you still prefer a PE-wide change? If so I'll add it to v2.
As per above - yes, I think we want PE-wide adjustments.
Jan
Am Dienstag, 6. Oktober 2026, 10:19:59 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
> On 06.10.2026 09:44, Tobias Deiminger wrote:
> > Am Freitag, 2. Oktober 2026, 14:46:43 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
> >> On 28.09.2026 11:01, Tobias Deiminger wrote:
> >>> ...
> > The "should" leaves the odd possibility that an object could have a non-zero
> > optional header. objcopy can create such odd, yet valid objects today. Whether
> > it makes sense I don't know.
>
> Imo such uses of "should" often are an attempt to be kind, i.e. to avoid
> words like "must" or "has to". Hence I question whether object files with
> the field non-zero actually are valid.
>
> > However, bigobj has a distinct file header
> >
> > // bigobj doesn't use COFF File Header, but a special one
> > typedef struct ANON_OBJECT_HEADER_BIGOBJ {
> > ...
> > // no SizeOfOptionalHeader field here at all
> > ...
> > }
> >
> > That means bigobj can definitely not have an optional header, because the
> > bigobj file header has no field to declare room for an optional header.
> >
> >> That is, shouldn't e.g. pe-<arch>.c and pei-<arch>.c control
> >> (differently) what coff-<arch>.c (aided by coffcode.h) puts in place?
> >> (Really this may not need any change to pe{,i}-<arch>.c, but may be
> >> possible to control via use of COFF_IMAGE_WITH_PE directly in
> >> coff-<arch>.c / coffcode.h. And then uniformly for bigobj and "normal"
> >> objects. Likely the !COFF_WITH_PE case also would need leaving alone
> >> altogether, just to avoid breaking anything there.)
> >
> > I wouldn't have been confident enough to change all PE formats in BFD so that
> > "should be zero" for us means "we don't support an optional header". Thus kept
> > the scope local to bigobj.
> >
> > Do you still prefer a PE-wide change? If so I'll add it to v2.
>
> As per above - yes, I think we want PE-wide adjustments.
Tried, works basically as you've sketched it.
I'd like to suggest a variant. How about controlling only EXEC_P generically
with COFF_WITH_PE/COFF_IMAGE_WITH_PE as you suggested, but keep AOUTSZ
format-specific?
This has the effect that existing odd PE objects with OptionalHeader remain
readable while bfd will no longer write any PE object with OptionalHeader,
avoiding the corruption.
Tobias
On 07.10.2026 12:11, Tobias Deiminger wrote:
> Am Dienstag, 6. Oktober 2026, 10:19:59 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
>> On 06.10.2026 09:44, Tobias Deiminger wrote:
>>> Am Freitag, 2. Oktober 2026, 14:46:43 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
>>>> On 28.09.2026 11:01, Tobias Deiminger wrote:
>>>>> ...
>>> The "should" leaves the odd possibility that an object could have a non-zero
>>> optional header. objcopy can create such odd, yet valid objects today. Whether
>>> it makes sense I don't know.
>>
>> Imo such uses of "should" often are an attempt to be kind, i.e. to avoid
>> words like "must" or "has to". Hence I question whether object files with
>> the field non-zero actually are valid.
>>
>>> However, bigobj has a distinct file header
>>>
>>> // bigobj doesn't use COFF File Header, but a special one
>>> typedef struct ANON_OBJECT_HEADER_BIGOBJ {
>>> ...
>>> // no SizeOfOptionalHeader field here at all
>>> ...
>>> }
>>>
>>> That means bigobj can definitely not have an optional header, because the
>>> bigobj file header has no field to declare room for an optional header.
>>>
>>>> That is, shouldn't e.g. pe-<arch>.c and pei-<arch>.c control
>>>> (differently) what coff-<arch>.c (aided by coffcode.h) puts in place?
>>>> (Really this may not need any change to pe{,i}-<arch>.c, but may be
>>>> possible to control via use of COFF_IMAGE_WITH_PE directly in
>>>> coff-<arch>.c / coffcode.h. And then uniformly for bigobj and "normal"
>>>> objects. Likely the !COFF_WITH_PE case also would need leaving alone
>>>> altogether, just to avoid breaking anything there.)
>>>
>>> I wouldn't have been confident enough to change all PE formats in BFD so that
>>> "should be zero" for us means "we don't support an optional header". Thus kept
>>> the scope local to bigobj.
>>>
>>> Do you still prefer a PE-wide change? If so I'll add it to v2.
>>
>> As per above - yes, I think we want PE-wide adjustments.
>
> Tried, works basically as you've sketched it.
>
> I'd like to suggest a variant. How about controlling only EXEC_P generically
> with COFF_WITH_PE/COFF_IMAGE_WITH_PE as you suggested, but keep AOUTSZ
> format-specific?
>
> This has the effect that existing odd PE objects with OptionalHeader remain
> readable while bfd will no longer write any PE object with OptionalHeader,
> avoiding the corruption.
Why not, if suitably explained in the description.
Jan
@@ -1028,9 +1028,9 @@ const bfd_target
BFD_ENDIAN_LITTLE, /* Data byte order is little. */
BFD_ENDIAN_LITTLE, /* Header byte order is little. */
- (HAS_RELOC | EXEC_P /* Object flags. */
- | HAS_LINENO | HAS_DEBUG
- | HAS_SYMS | HAS_LOCALS | WP_TEXT | D_PAGED | BFD_COMPRESS | BFD_DECOMPRESS),
+ (HAS_RELOC | HAS_LINENO /* Object flags. */
+ | HAS_DEBUG | HAS_SYMS
+ | HAS_LOCALS | WP_TEXT | D_PAGED | BFD_COMPRESS | BFD_DECOMPRESS),
(SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_RELOC /* Section flags. */
#ifdef COFF_WITH_PE
@@ -801,9 +801,9 @@ const bfd_target
BFD_ENDIAN_LITTLE, /* data byte order is little */
BFD_ENDIAN_LITTLE, /* header byte order is little */
- (HAS_RELOC | EXEC_P | /* object flags */
- HAS_LINENO | HAS_DEBUG |
- HAS_SYMS | HAS_LOCALS | WP_TEXT | D_PAGED | BFD_COMPRESS | BFD_DECOMPRESS ),
+ (HAS_RELOC | HAS_LINENO | /* object flags */
+ HAS_DEBUG | HAS_SYMS |
+ HAS_LOCALS | WP_TEXT | D_PAGED | BFD_COMPRESS | BFD_DECOMPRESS ),
(SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_RELOC /* section flags */
#ifdef COFF_WITH_PE
@@ -984,9 +984,9 @@ const bfd_target
BFD_ENDIAN_LITTLE, /* Data byte order is little. */
BFD_ENDIAN_LITTLE, /* Header byte order is little. */
- (HAS_RELOC | EXEC_P /* Object flags. */
- | HAS_LINENO | HAS_DEBUG
- | HAS_SYMS | HAS_LOCALS | WP_TEXT | D_PAGED | BFD_COMPRESS | BFD_DECOMPRESS),
+ (HAS_RELOC | HAS_LINENO /* Object flags. */
+ | HAS_DEBUG | HAS_SYMS
+ | HAS_LOCALS | WP_TEXT | D_PAGED | BFD_COMPRESS | BFD_DECOMPRESS),
(SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_RELOC /* Section flags. */
#if defined(COFF_WITH_PE)
@@ -3055,10 +3055,23 @@ coff_compute_section_file_positions (bfd * abfd)
}
#endif
- if (bfd_get_start_address (abfd))
- /* A start address may have been added to the original file. In this
- case it will need an optional header to record it. */
- abfd->flags |= EXEC_P;
+ if (bfd_applicable_file_flags (abfd) & EXEC_P)
+ {
+ /* A start address may have been added to the original file. In this
+ case it will need an optional header to record it. */
+ if (bfd_get_start_address (abfd))
+ abfd->flags |= EXEC_P;
+ }
+ else if (abfd->flags & EXEC_P)
+ {
+ /* ld or auto-promotion may have requested the executable flag even if
+ not supported by the target format. */
+ _bfd_error_handler
+ /* xgettext:c-format */
+ (_("%pB: warning: %s cannot be executable or have a start address"),
+ abfd, bfd_get_target (abfd));
+ abfd->flags &= ~EXEC_P;
+ }
if (abfd->flags & EXEC_P)
sofar += bfd_coff_aoutsz (abfd);
@@ -5940,7 +5953,7 @@ static const bfd_coff_backend_data bigobj_swap_table =
coff_SWAP_lineno_out, coff_SWAP_reloc_out,
coff_bigobj_swap_filehdr_out, coff_SWAP_aouthdr_out,
coff_SWAP_scnhdr_out,
- FILHSZ_BIGOBJ, AOUTSZ, SCNHSZ, SYMESZ_BIGOBJ, AUXESZ_BIGOBJ,
+ FILHSZ_BIGOBJ, 0, SCNHSZ, SYMESZ_BIGOBJ, AUXESZ_BIGOBJ,
RELSZ, LINESZ, FILNMLEN_BIGOBJ,
true,
COFF_DEFAULT_LONG_SECTION_NAMES,