Skip to content

Commit 1dfd266

Browse files
committed
Harden map file parsing and document PE vs ELF asymmetry
ParseMapFile: - Anchor parsing to the Publics by Value column header so a same-named entry in the Static symbols section can't shadow or collide with a public BORINGSSL_bcm_* marker. - Fail fast with a descriptive error if the map file does not contain a Publics by Value section, instead of producing a downstream 'symbol not found in map file' error. - Expand the function doc with the expected map-file grammar. break-hash.go: - Add a comment on doPE explaining why it does not need the 256-byte prefix uniqueness check that doELF performs: the linker map plus the PE section table already yield an exact on-disk offset for BORINGSSL_bcm_text_start, so the ELF-era workaround is unnecessary.
1 parent f96e4da commit 1dfd266

3 files changed

Lines changed: 59 additions & 2 deletions

File tree

util/fipstools/break-hash.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ func doELF(objectBytes []byte) (int, []byte, error) {
104104
}
105105

106106
func doPE(objectBytes []byte, mapPath string) (int, []byte, error) {
107+
// Unlike doELF, we do not need to search the raw file for a unique
108+
// 256-byte prefix of the module text to find its file offset: the map
109+
// file gives us the exact virtual address of BORINGSSL_bcm_text_start,
110+
// and the PE section table lets us translate that directly to a file
111+
// offset. The uniqueness check doELF performs is a workaround for the
112+
// fact that ELF symbol values alone do not pinpoint the on-disk location
113+
// of the module; the PE path does not have that limitation.
107114
symbolAddrs, err := fipscommon.ParseMapFile(mapPath)
108115
if err != nil {
109116
return 0, nil, err

util/fipstools/fipscommon/pe.go

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,50 @@ type PEInfo struct {
2121

2222
// ParseMapFile reads a Windows linker map file and returns a map of
2323
// BORINGSSL_bcm_* symbol names to their Rva+Base addresses.
24+
//
25+
// MSVC-style map files (also produced by lld-link with /MAP:) contain several
26+
// sections. Symbol addresses we care about live under a column-header line of
27+
// the form:
28+
//
29+
// Address Publics by Value Rva+Base Lib:Object
30+
//
31+
// followed by rows of the form:
32+
//
33+
// SSSS:OOOOOOOO name RRRRRRRRRRRRRRRR [f] Lib:Object
34+
//
35+
// A later "Static symbols" heading begins a section that reuses the row format,
36+
// so anchoring parsing to the Publics header avoids accidentally picking up a
37+
// same-named static symbol from some other translation unit. The
38+
// BORINGSSL_bcm_* markers are `extern` and therefore only ever appear in
39+
// Publics by Value in practice, but the anchor makes the intent explicit and
40+
// the parser less fragile.
2441
func ParseMapFile(mapPath string) (map[string]uint64, error) {
2542
data, err := os.ReadFile(mapPath)
2643
if err != nil {
2744
return nil, fmt.Errorf("failed to read map file: %s", err.Error())
2845
}
2946

3047
symbols := make(map[string]uint64)
31-
// Symbol lines have format: SSSS:OOOOOOOO name RRRRRRRRRRRRRRRR Lib:Object
48+
inPublics := false
49+
sawPublicsHeading := false
3250
for _, line := range strings.Split(string(data), "\n") {
51+
trimmed := strings.TrimSpace(line)
52+
// Section headings are column-header / label lines with no leading
53+
// address column. Detect the Publics by Value column-header line
54+
// (which also contains the word "Address") and the Static symbols
55+
// label that terminates the public section.
56+
if strings.Contains(trimmed, "Publics by Value") {
57+
inPublics = true
58+
sawPublicsHeading = true
59+
continue
60+
}
61+
if trimmed == "Static symbols" {
62+
inPublics = false
63+
continue
64+
}
65+
if !inPublics {
66+
continue
67+
}
3368
fields := strings.Fields(line)
3469
if len(fields) < 3 || !strings.Contains(fields[0], ":") {
3570
continue
@@ -48,6 +83,13 @@ func ParseMapFile(mapPath string) (map[string]uint64, error) {
4883
symbols[name] = rvaBase
4984
}
5085

86+
// Guard against silently-malformed map files: if we never saw the
87+
// expected heading the caller will get confusing "symbol not found"
88+
// errors downstream. Surface the real problem here instead.
89+
if !sawPublicsHeading {
90+
return nil, fmt.Errorf("map file %q does not contain a \"Publics by Value\" section; is this an MSVC-style linker map?", mapPath)
91+
}
92+
5193
return symbols, nil
5294
}
5395

@@ -85,7 +127,11 @@ func (p *PEInfo) ResolveSymbolFileOffset(symbolAddrs map[string]uint64, name str
85127
for _, s := range p.File.Sections {
86128
start := uint64(s.VirtualAddress)
87129
if rva >= start && rva < start+uint64(s.VirtualSize) {
88-
return rva - start + uint64(s.Offset), nil
130+
offsetInSection := rva - start
131+
if offsetInSection >= uint64(s.Size) {
132+
return 0, fmt.Errorf("RVA 0x%x for %q is in zero-fill/BSS region of section %s (offset 0x%x exceeds raw data size 0x%x)", rva, name, s.Name, offsetInSection, s.Size)
133+
}
134+
return offsetInSection + uint64(s.Offset), nil
89135
}
90136
}
91137
return 0, fmt.Errorf("RVA 0x%x for %q not found in any PE section", rva, name)

util/fipstools/inject_hash/inject_hash.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,10 @@ func do(outPath, oInput string, arInput string, appleOS bool, windowsOS bool, ma
354354
var isStatic bool
355355
var perm os.FileMode
356356

357+
if appleOS && windowsOS {
358+
return fmt.Errorf("-apple and -windows are mutually exclusive")
359+
}
360+
357361
if windowsOS && len(mapFile) == 0 {
358362
return fmt.Errorf("-map is required when -windows is set")
359363
}

0 commit comments

Comments
 (0)