Skip to content

[LLD][COFF] Fix out-of-bounds write when filling gaps with INT3 in code sections #180411

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.

Sign up for GitHub

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

Merged
aganea merged 1 commit into from
Feb 10, 2026
Merged

[LLD][COFF] Fix out-of-bounds write when filling gaps with INT3 in code sections #180411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lld/COFF/Writer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2619,12 +2619,17 @@ void Writer::writeSections() {
if ((sec->header.Characteristics & IMAGE_SCN_CNT_CODE) &&
(ctx.config.machine == AMD64 || ctx.config.machine == I386)) {
uint32_t prevEnd = 0;
uint32_t rawSize = sec->getRawSize();
for (Chunk *c : sec->chunks) {
uint32_t off = c->getRVA() - sec->getRVA();
// Chunks without data (e.g., .bss) have virtual addresses beyond
// rawSize; stop filling when we reach the end of raw data.
if (off >= rawSize)

@aganea aganea Feb 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this check could prevent from filling the gap between the last .text chunk and the .bss, if the file alignement is larger than the last .text chunk?

@wbenny wbenny Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the memset after the loop handles that.

@maj113 maj113 May 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this exposed a new bug I guess:

clang_release_ble/DM40Native.exe //20.1.8
  FileSize=55550 EP=0xd8b5 SizeOfImage=0xd8fe SizeOfHeaders=0x198
  [.data] VA=0x198 VSize=0xd766 RawOff=0x198 RawSz=0xd766 Chars=0xe0000020  VSize-RawSz=0

clang_release_ble/DM40Native_broken.exe //trunk
  FileSize=55710 EP=0xd949 SizeOfImage=0xd9a4 SizeOfHeaders=0x198
  [.data] VA=0x198 VSize=0xd80c RawOff=0x198 RawSz=0xd806 Chars=0xe0000020  VSize-RawSz=6

clang_release_bt/DM40Native.exe //20.1.8
  FileSize=52542 EP=0xccc2 SizeOfImage=0xcd3e SizeOfHeaders=0x198
  [.data] VA=0x198 VSize=0xcba6 RawOff=0x198 RawSz=0xcba6 Chars=0xe0000020  VSize-RawSz=0

clang_release_bt/DM40Native_broken.exe //trunk
  FileSize=52718 EP=0xcd6a SizeOfImage=0xcdf4 SizeOfHeaders=0x198
  [.data] VA=0x198 VSize=0xcc5c RawOff=0x198 RawSz=0xcc56 Chars=0xe0000020  VSize-RawSz=6

PE loader rejects the exe

seems to be due to:

if (c->hasData)
    rawSize = alignTo(virtualSize, config->fileAlign);

since bss is not handled in this block it gets excluded by this, locally for me this fixed it but this probably isn't the correct solution since filealign 2 and 4 would still probably be outside the rawSize:

if (config->fileAlign == 1 && rawSize < virtualSize)
    rawSize = virtualSize;

so maybe something like this would be better:

if (rawSize < alignTo(virtualSize, config->fileAlign))
    rawSize = alignTo(virtualSize, config->fileAlign);

@mstorsjo mstorsjo May 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this issue that you're observing is related to this commit? As far as I can see, this change shouldn't affect the final layout of the PE image (which can cause the image to be rejected), only what is filled in it.

Can you file a separate issue about the problem you're seeing, so we can try to track it down? Ideally, can you provider a reproducer? If you add -reproduce:repro.tar to the link command, it'll produce a file repro.tar which contains everything required for rerunning the link command. That would allow others to pinpoint exactly which LLD change is causing the PE loader to reject it. (If linking through the compiler driver in mingw mode, pass -Wl,--reproduce=repro.tar instead.)

break;
memset(secBuf + prevEnd, 0xCC, off - prevEnd);
prevEnd = off + c->getSize();
prevEnd = std::min(off + static_cast<uint32_t>(c->getSize()), rawSize);
}
memset(secBuf + prevEnd, 0xCC, sec->getRawSize() - prevEnd);
memset(secBuf + prevEnd, 0xCC, rawSize - prevEnd);
}

parallelForEach(sec->chunks, [&](Chunk *c) {
Expand Down
60 changes: 60 additions & 0 deletions lld/test/COFF/merge-bss-text-filealign1.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# REQUIRES: x86
# Test that merging .bss into .text with /FILEALIGN:1 doesn't crash.
#
# RUN: yaml2obj %s -o %t.obj
# RUN: lld-link /out:%t.exe /entry:main /subsystem:console \
# RUN: /merge:.bss=.text /filealign:1 %t.obj
# RUN: llvm-readobj --sections %t.exe | FileCheck %s

# CHECK: Name: .text
# CHECK-NEXT: VirtualSize: 0x104
# CHECK: RawDataSize:
# CHECK-NOT: Name: .bss

--- !COFF
header:
Machine: IMAGE_FILE_MACHINE_AMD64
Characteristics: [ ]
sections:
- Name: .text
Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
Alignment: 4
SectionData: 'C3'
SizeOfRawData: 1
- Name: .bss
Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
Alignment: 4
SectionData: ''
SizeOfRawData: 256
symbols:
- Name: .text
Value: 0
SectionNumber: 1
SimpleType: IMAGE_SYM_TYPE_NULL
ComplexType: IMAGE_SYM_DTYPE_NULL
StorageClass: IMAGE_SYM_CLASS_STATIC
SectionDefinition:
Length: 1
NumberOfRelocations: 0
NumberOfLinenumbers: 0
CheckSum: 0
Number: 1
- Name: .bss
Value: 0
SectionNumber: 2
SimpleType: IMAGE_SYM_TYPE_NULL
ComplexType: IMAGE_SYM_DTYPE_NULL
StorageClass: IMAGE_SYM_CLASS_STATIC
SectionDefinition:
Length: 256
NumberOfRelocations: 0
NumberOfLinenumbers: 0
CheckSum: 0
Number: 2
- Name: main
Value: 0
SectionNumber: 1
SimpleType: IMAGE_SYM_TYPE_NULL
ComplexType: IMAGE_SYM_DTYPE_NULL
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
...