inject_hash: Support finding the marker symbols in .symtab or .dynsym When testing I2f1999679b1987664da4350d86ac149e52d44303, I found that some build configuration that injected -Wl,--strip-all into the build in some way that I could not figure out how to undo. That meant that the sample shared object lost its marker symbols for where to compute the hash. To work around that, update inject_hash to also check dynamic symbols for the markers. For the markers to end up in there when built via delocate, they must be .globl. However, if we then made them visible[*], we risk polluting the dynamic symbol table of the final binary, because we don't know whether the consumer controlled symbol visibility pre-link with -fvisibility=hidden or post-link (e.g. with a symbol version script). To account for this, they're marked hidden, which actually keeps them out of the dynamic symbol table anyway. But it allows objcopy --set-symbol-visibility to catch and upgrade them in the sample shared library, in builds where an errant -Wl,--strip-all impedes us. (Builds without this problem don't need to both, because the symbol will end up in .symtab.) That, in turn, requires us to stop treating these symbols as synthesized local symbols, so we get a little extra complexity there. [*] To be visible, they actually need to be protected visibility rather than default visibility. Otherwise the assembler complains that we aren't GOT-indirecting all accesses to them. Change-Id: Ia677ac274fc3e55a2e3cc7832f6abae78cc9589b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/86088 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/util/fipstools/delocate/delocate.go b/util/fipstools/delocate/delocate.go index fd8b25b..3f14127 100644 --- a/util/fipstools/delocate/delocate.go +++ b/util/fipstools/delocate/delocate.go
@@ -1447,6 +1447,12 @@ commentIndicator = "//" } + // These symbols will be synthesized below as global symbols. Mark them as + // known, so we will rewrite them to their local target name and avoid a + // relocation. + symbols["BORINGSSL_bcm_text_start"] = struct{}{} + symbols["BORINGSSL_bcm_text_end"] = struct{}{} + d := &delocation{ symbols: symbols, processor: processor, @@ -1466,7 +1472,13 @@ } 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)) + // Mark BORINGSSL_bcm_text_start as global, so that our tools can more reliably find it, + // but hidden so it does not pollute downstream consumers' dynamic symbol tables. This + // is primarily a hook for objcopy to upgrade to visible, if needed to sample the hash. + w.WriteString(".globl BORINGSSL_bcm_text_start\n") + w.WriteString(".hidden BORINGSSL_bcm_text_start\n") w.WriteString("BORINGSSL_bcm_text_start:\n") + w.WriteString(localTargetName("BORINGSSL_bcm_text_start") + ":\n") for _, input := range inputs { if err := d.processInput(input); err != nil { @@ -1476,7 +1488,10 @@ w.WriteString(".text\n") w.WriteString(fmt.Sprintf(".loc %d 2 0\n", maxObservedFileNumber+1)) + w.WriteString(".globl BORINGSSL_bcm_text_end\n") + w.WriteString(".hidden BORINGSSL_bcm_text_end\n") w.WriteString("BORINGSSL_bcm_text_end:\n") + w.WriteString(localTargetName("BORINGSSL_bcm_text_end") + ":\n") // Emit redirector functions. Each is a single jump instruction. var redirectorNames []string @@ -1819,8 +1834,7 @@ } func isSynthesized(symbol string) bool { - return strings.HasSuffix(symbol, "_bss_get") || - strings.HasPrefix(symbol, "BORINGSSL_bcm_text_") + return strings.HasSuffix(symbol, "_bss_get") } func redirectorName(symbol string) string {
diff --git a/util/fipstools/delocate/testdata/aarch64-Basic/out.s b/util/fipstools/delocate/testdata/aarch64-Basic/out.s index de147ca..aa03451 100644 --- a/util/fipstools/delocate/testdata/aarch64-Basic/out.s +++ b/util/fipstools/delocate/testdata/aarch64-Basic/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .type foo, %function .globl foo .Lfoo_local_target: @@ -38,7 +41,7 @@ // GOT load of synthesized symbol. // WAS adrp x0, :got:BORINGSSL_bcm_text_start - adr x0, BORINGSSL_bcm_text_start + adr x0, .LBORINGSSL_bcm_text_start_local_target // WAS ldr x0, [x0, :got_lo12:BORINGSSL_bcm_text_start] // Address load @@ -176,7 +179,10 @@ .size bss_symbol, 4 .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .p2align 2 .hidden bcm_redirector_p224_point_add .type bcm_redirector_p224_point_add, @function
diff --git a/util/fipstools/delocate/testdata/generic-FileDirectives/out.s b/util/fipstools/delocate/testdata/generic-FileDirectives/out.s index b2e3daa..e5aa58f 100644 --- a/util/fipstools/delocate/testdata/generic-FileDirectives/out.s +++ b/util/fipstools/delocate/testdata/generic-FileDirectives/out.s
@@ -1,7 +1,10 @@ .text .file 1002 "inserted_by_delocate.c" md5 0x00000000000000000000000000000000 .loc 1002 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .file 10 "some/path/file.c" "file.c" .file 1000 "some/path/file2.c" "file2.c" .file 1001 "some/path/file_with_md5.c" "other_name.c" md5 0x5eba7844df6449a7f2fff1556fe7ba8d239f8e2f @@ -10,7 +13,10 @@ movq %rax, %rbx .text .loc 1002 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type BORINGSSL_bcm_text_hash, @object .size BORINGSSL_bcm_text_hash, 32 BORINGSSL_bcm_text_hash:
diff --git a/util/fipstools/delocate/testdata/x86_64-BSS/out.s b/util/fipstools/delocate/testdata/x86_64-BSS/out.s index 4a5be91..cda5830 100644 --- a/util/fipstools/delocate/testdata/x86_64-BSS/out.s +++ b/util/fipstools/delocate/testdata/x86_64-BSS/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .text movq %rax, %rax @@ -59,7 +62,10 @@ .quad 0 .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type aes_128_ctr_generic_storage_bss_get, @function aes_128_ctr_generic_storage_bss_get: leaq aes_128_ctr_generic_storage(%rip), %rax
diff --git a/util/fipstools/delocate/testdata/x86_64-Basic/out.s b/util/fipstools/delocate/testdata/x86_64-Basic/out.s index b0b90b0..b10b0ae 100644 --- a/util/fipstools/delocate/testdata/x86_64-Basic/out.s +++ b/util/fipstools/delocate/testdata/x86_64-Basic/out.s
@@ -1,7 +1,10 @@ .text .file 2 "inserted_by_delocate.c" .loc 2 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: # Most instructions and lines should pass unaltered. This is made up of # copy-and-pasted bits of compiler output and likely does not actually # run. @@ -59,7 +62,10 @@ .uleb128 .foo-1-.bar .text .loc 2 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type BORINGSSL_bcm_text_hash, @object .size BORINGSSL_bcm_text_hash, 32 BORINGSSL_bcm_text_hash:
diff --git a/util/fipstools/delocate/testdata/x86_64-GOTRewrite/out.s b/util/fipstools/delocate/testdata/x86_64-GOTRewrite/out.s index f77f907..727b268 100644 --- a/util/fipstools/delocate/testdata/x86_64-GOTRewrite/out.s +++ b/util/fipstools/delocate/testdata/x86_64-GOTRewrite/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .text .Lfoo_local_target: foo: @@ -104,7 +107,7 @@ # Synthesized symbols do not use the GOT. # WAS movq BORINGSSL_bcm_text_start@GOTPCREL(%rip), %r11 - leaq BORINGSSL_bcm_text_start(%rip), %r11 + leaq .LBORINGSSL_bcm_text_start_local_target(%rip), %r11 # WAS movq foobar_bss_get@GOTPCREL(%rip), %r11 leaq foobar_bss_get(%rip), %r11 @@ -232,7 +235,10 @@ .comm foobar,64,32 .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type foobar_bss_get, @function foobar_bss_get: leaq foobar(%rip), %rax
diff --git a/util/fipstools/delocate/testdata/x86_64-LabelRewrite/out.s b/util/fipstools/delocate/testdata/x86_64-LabelRewrite/out.s index abc0b71..936405b 100644 --- a/util/fipstools/delocate/testdata/x86_64-LabelRewrite/out.s +++ b/util/fipstools/delocate/testdata/x86_64-LabelRewrite/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .type foo, @function .globl foo .Lfoo_local_target: @@ -153,7 +156,10 @@ vpcmpneqq .Llabel_BCM_1(%rip){1to8}, %zmm1, %k0 .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type bcm_redirector_memcpy, @function bcm_redirector_memcpy: jmp memcpy@PLT
diff --git a/util/fipstools/delocate/testdata/x86_64-LargeMemory/out.s b/util/fipstools/delocate/testdata/x86_64-LargeMemory/out.s index ebdf28b..adf2e84 100644 --- a/util/fipstools/delocate/testdata/x86_64-LargeMemory/out.s +++ b/util/fipstools/delocate/testdata/x86_64-LargeMemory/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .text # PIC function call @@ -40,7 +43,10 @@ .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .Lboringssl_got_delta: .quad _GLOBAL_OFFSET_TABLE_-.Lboringssl_got_delta .Lboringssl_got_h:
diff --git a/util/fipstools/delocate/testdata/x86_64-Sections/out.s b/util/fipstools/delocate/testdata/x86_64-Sections/out.s index c35826b..facc7cb 100644 --- a/util/fipstools/delocate/testdata/x86_64-Sections/out.s +++ b/util/fipstools/delocate/testdata/x86_64-Sections/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: # .text stays in .text .text movq %rax, %rax @@ -46,7 +49,10 @@ .long .L3 .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type BORINGSSL_bcm_text_hash, @object .size BORINGSSL_bcm_text_hash, 32 BORINGSSL_bcm_text_hash:
diff --git a/util/fipstools/delocate/testdata/x86_64-ThreeArg/out.s b/util/fipstools/delocate/testdata/x86_64-ThreeArg/out.s index 2e4ba11..d14fdf3 100644 --- a/util/fipstools/delocate/testdata/x86_64-ThreeArg/out.s +++ b/util/fipstools/delocate/testdata/x86_64-ThreeArg/out.s
@@ -1,7 +1,10 @@ .text .file 1 "inserted_by_delocate.c" .loc 1 1 0 +.globl BORINGSSL_bcm_text_start +.hidden BORINGSSL_bcm_text_start BORINGSSL_bcm_text_start: +.LBORINGSSL_bcm_text_start_local_target: .type foo, @function .globl foo .Lfoo_local_target: @@ -33,7 +36,10 @@ .quad -2404814165548301886 # 0xdea06241f7aa81c2 .text .loc 1 2 0 +.globl BORINGSSL_bcm_text_end +.hidden BORINGSSL_bcm_text_end BORINGSSL_bcm_text_end: +.LBORINGSSL_bcm_text_end_local_target: .type BORINGSSL_bcm_text_hash, @object .size BORINGSSL_bcm_text_hash, 32 BORINGSSL_bcm_text_hash:
diff --git a/util/fipstools/inject_hash/inject_hash.go b/util/fipstools/inject_hash/inject_hash.go index fdd6f4a..2785029 100644 --- a/util/fipstools/inject_hash/inject_hash.go +++ b/util/fipstools/inject_hash/inject_hash.go
@@ -124,7 +124,12 @@ var textStart, textEnd, rodataStart, rodataEnd *uint64 + // Look for symbols in either .symtab or .dynsym. Some build configurations + // strip way .symtab. symbols, err := object.Symbols() + if err == elf.ErrNoSymbols { + symbols, err = object.DynamicSymbols() + } if err != nil { return errors.New("failed to parse symbols: " + err.Error()) }