Skip to content

Commit

Permalink
Read fips scope marker addresses using adrp instead of adr to increas…
Browse files Browse the repository at this point in the history
…e reach (#1332)

When including the new Ed25519 implementations in the FIPS build the follow error occurred:
bcm.c:47662: Error: pc-relative address offset out of range
The reason is that we get the address of e.g. BORINGSSL_bcm_text_end using adr like this:
adr x4, BORINGSSL_bcm_text_end
But since the size of the fips module has slowly increased the last few years the amount of memory we attempt to jump with adr runs over the reach of the instruction (about 1MB or so).

The solution is to no longer consider BORINGSSL_bcm_text_[end,start] for special and use a redirector function to resolve their addresses.
  • Loading branch information
torben-hansen authored Dec 5, 2023
1 parent bc9b35c commit 43164b7
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 9 deletions.
49 changes: 40 additions & 9 deletions util/fipstools/delocate/delocate.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ func (d *delocation) loadAarch64Address(statement *node32, targetReg string, sym

_, isKnown := d.symbols[symbol]
isLocal := strings.HasPrefix(symbol, ".L")
if isKnown || isLocal || isSynthesized(symbol) {
if isKnown || isLocal || isSynthesized(symbol, aarch64) {
if isLocal {
symbol = d.mapLocalSymbol(symbol)
} else if isKnown {
Expand Down Expand Up @@ -566,10 +566,17 @@ func (d *delocation) processAarch64Instruction(statement, instruction *node32) (
symbol, offset, _, didChange, symbolIsLocal, _ := d.parseMemRef(arg.up)
changed = didChange

if _, knownSymbol := d.symbols[symbol]; knownSymbol {
if isFipsScopeMarkers(symbol) {
// fips scope markers are known. But they challenge the adr
// reach, so go through GOT via an adrp outside the scope.
redirector := redirectorName(symbol)
d.redirectors[symbol] = redirector
symbol = redirector
changed = true
} else if _, knownSymbol := d.symbols[symbol]; knownSymbol {
symbol = localTargetName(symbol)
changed = true
} else if !symbolIsLocal && !isSynthesized(symbol) {
} else if !symbolIsLocal && !isSynthesized(symbol, aarch64) {
redirector := redirectorName(symbol)
d.redirectors[symbol] = redirector
symbol = redirector
Expand Down Expand Up @@ -981,7 +988,7 @@ Args:
} else if _, knownSymbol := d.symbols[symbol]; knownSymbol {
symbol = localTargetName(symbol)
changed = true
} else if !symbolIsLocal && !isSynthesized(symbol) && len(section) == 0 {
} else if !symbolIsLocal && !isSynthesized(symbol, ppc64le) && len(section) == 0 {
changed = true
d.redirectors[symbol] = redirectorName(symbol)
symbol = redirectorName(symbol)
Expand Down Expand Up @@ -1421,7 +1428,7 @@ Args:
if _, knownSymbol := d.symbols[symbol]; knownSymbol {
symbol = localTargetName(symbol)
changed = true
} else if !symbolIsLocal && !isSynthesized(symbol) {
} else if !symbolIsLocal && !isSynthesized(symbol, x86_64) {
// Unknown symbol via PLT is an
// out-call from the module, e.g.
// memcpy.
Expand All @@ -1443,7 +1450,7 @@ Args:
if _, knownSymbol := d.symbols[symbol]; knownSymbol {
symbol = localTargetName(symbol)
changed = true
} else if !isSynthesized(symbol) {
} else if !isSynthesized(symbol, x86_64) {
useGOT = true
}

Expand Down Expand Up @@ -1834,6 +1841,13 @@ func transform(w stringWriter, inputs []inputFile) error {
}
w.WriteString(fmt.Sprintf(".file %d \"inserted_by_delocate.c\"%s\n", maxObservedFileNumber+1, fileTrailing))
w.WriteString(fmt.Sprintf(".loc %d 1 0\n", maxObservedFileNumber+1))
if d.processor == aarch64 {
// Grab the address of BORINGSSL_bcm_test_[start,end] via a relocation
// from a redirector function. For this to work, need to add the markers
// to the symbol table.
w.WriteString(fmt.Sprintf(".global BORINGSSL_bcm_text_start\n"))
w.WriteString(fmt.Sprintf(".type BORINGSSL_bcm_text_start, @function\n"))
}
w.WriteString("BORINGSSL_bcm_text_start:\n")

for _, input := range inputs {
Expand All @@ -1844,6 +1858,10 @@ func transform(w stringWriter, inputs []inputFile) error {

w.WriteString(".text\n")
w.WriteString(fmt.Sprintf(".loc %d 2 0\n", maxObservedFileNumber+1))
if d.processor == aarch64 {
w.WriteString(fmt.Sprintf(".global BORINGSSL_bcm_text_end\n"))
w.WriteString(fmt.Sprintf(".type BORINGSSL_bcm_text_end, @function\n"))
}
w.WriteString("BORINGSSL_bcm_text_end:\n")

// Emit redirector functions. Each is a single jump instruction.
Expand Down Expand Up @@ -2256,10 +2274,23 @@ func localEntryName(name string) string {
return ".L" + name + "_local_entry"
}

func isSynthesized(symbol string) bool {
return strings.HasSuffix(symbol, "_bss_get") ||
func isSynthesized(symbol string, processor processorType) bool {
SymbolisSynthesized := strings.HasSuffix(symbol, "_bss_get") ||
symbol == "OPENSSL_ia32cap_get" ||
strings.HasPrefix(symbol, "BORINGSSL_bcm_text_")
symbol == "BORINGSSL_bcm_text_hash"

// While BORINGSSL_bcm_text_[start,end] are known symbols, on aarch64 we go
// through the GOT because adr doesn't have adequate reach.
if (processor != aarch64) {
SymbolisSynthesized = SymbolisSynthesized || strings.HasPrefix(symbol, "BORINGSSL_bcm_text_")
}

return SymbolisSynthesized
}

func isFipsScopeMarkers(symbol string) bool {
return symbol == "BORINGSSL_bcm_text_start" ||
symbol == "BORINGSSL_bcm_text_end"
}

func redirectorName(symbol string) string {
Expand Down
4 changes: 4 additions & 0 deletions util/fipstools/delocate/testdata/aarch64-Basic/in.s
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ foo:
add x22, sp, #(13*32)+96
add x22, sp, #(13*32)+96*32

// Ensure BORINGSSL_bcm_text_[end,start] are loaded through GOT
adrp x4, :got:BORINGSSL_bcm_text_start
adrp x5, :got:BORINGSSL_bcm_text_end

local_function:

// BSS data
Expand Down
40 changes: 40 additions & 0 deletions util/fipstools/delocate/testdata/aarch64-Basic/out.s
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
.global BORINGSSL_bcm_text_start
.type BORINGSSL_bcm_text_start, @function
BORINGSSL_bcm_text_start:
.type foo, %function
.globl foo
Expand Down Expand Up @@ -141,6 +143,22 @@ foo:
add x22, sp, #(13*32)+96
add x22, sp, #(13*32)+96*32

// Ensure BORINGSSL_bcm_text_[end,start] are loaded through GOT
// WAS adrp x4, :got:BORINGSSL_bcm_text_start
sub sp, sp, 128
stp x0, lr, [sp, #-16]!
bl .Lboringssl_loadgot_BORINGSSL_bcm_text_start
mov x4, x0
ldp x0, lr, [sp], #16
add sp, sp, 128
// WAS adrp x5, :got:BORINGSSL_bcm_text_end
sub sp, sp, 128
stp x0, lr, [sp, #-16]!
bl .Lboringssl_loadgot_BORINGSSL_bcm_text_end
mov x5, x0
ldp x0, lr, [sp], #16
add sp, sp, 128

.Llocal_function_local_target:
local_function:

Expand All @@ -154,6 +172,8 @@ bss_symbol:
.size bss_symbol, 4
.text
.loc 1 2 0
.global BORINGSSL_bcm_text_end
.type BORINGSSL_bcm_text_end, @function
BORINGSSL_bcm_text_end:
.p2align 2
.hidden bcm_redirector_remote_function
Expand Down Expand Up @@ -190,6 +210,26 @@ bss_symbol_bss_get:
.cfi_endproc
.size bss_symbol_bss_get, .-bss_symbol_bss_get
.p2align 2
.hidden .Lboringssl_loadgot_BORINGSSL_bcm_text_end
.type .Lboringssl_loadgot_BORINGSSL_bcm_text_end, @function
.Lboringssl_loadgot_BORINGSSL_bcm_text_end:
.cfi_startproc
adrp x0, :got:BORINGSSL_bcm_text_end
ldr x0, [x0, :got_lo12:BORINGSSL_bcm_text_end]
ret
.cfi_endproc
.size .Lboringssl_loadgot_BORINGSSL_bcm_text_end, .-.Lboringssl_loadgot_BORINGSSL_bcm_text_end
.p2align 2
.hidden .Lboringssl_loadgot_BORINGSSL_bcm_text_start
.type .Lboringssl_loadgot_BORINGSSL_bcm_text_start, @function
.Lboringssl_loadgot_BORINGSSL_bcm_text_start:
.cfi_startproc
adrp x0, :got:BORINGSSL_bcm_text_start
ldr x0, [x0, :got_lo12:BORINGSSL_bcm_text_start]
ret
.cfi_endproc
.size .Lboringssl_loadgot_BORINGSSL_bcm_text_start, .-.Lboringssl_loadgot_BORINGSSL_bcm_text_start
.p2align 2
.hidden .Lboringssl_loadgot_stderr
.type .Lboringssl_loadgot_stderr, @function
.Lboringssl_loadgot_stderr:
Expand Down

0 comments on commit 43164b7

Please sign in to comment.