Figure out arm64 CFG issues
Categories
(Core :: Security, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox69 | --- | fixed |
People
(Reporter: away, Assigned: away)
References
Details
Attachments
(1 file)
Bug 1525588 disabled CFG on aarch64 because some xul functions aren't getting marked as valid call targets, so the browser fails to start at all.
We should still try to figure out what's going on and re-enable if possible.
rnk and pcc suggested stepping through markSymbolsWithRelocations in lld as a starting point.
Specifically, the problematic function is: xul!std::sys::windows::thread_local::on_tls_callback
(Or at least that's the first one we fail on, there may be more)
|
||
Comment 2•5 years ago
|
||
You might want to put your own GuardCFCheckFunctionPointer and GuardCFDispatchFunctionPointer functions in the load config directory and use these functions to build a list of indirect targets not found in the table.
|
|
||
Comment 3•5 years ago
|
||
Or even better - simply compare the ARM64 and the AMD64 lists...
(In reply to temp.stuff from comment #3)
Or even better - simply compare the ARM64 and the AMD64 lists...
The trouble is a bunch of the entries in dumpbin -loadconfig don't resolve to an address, e.g.:
Guard CF Function Table
Address
--------
[...snip...]
000000018000ABB0 msgpack_zone_init
000000018000AC10 msgpack_zone_new
000000018000AC80 msgpack_zone_free
000000018000B420
000000018000CEE0
000000018000D1E0
Do you know of another way to do the comparison?
|
|
||
Comment 5•5 years ago
|
||
dumpbin doesn't load PDBs for the modules it dumps AFAIK and therefore doesn't know about most symbols. The few it did display are probably exported.
You should either use a WinDbg script to get symbol names for the dumpbin output, or use WinDbg to both get the table entries and the corresponding symbols, or use the DbgEng API from C++ in your own client. (Or some other similar tool.)
Here's an example of how to do it from WinDbg:
0:000> dt xul!IMAGE_LOAD_CONFIG_DIRECTORY64 GuardCFFunctionTable GuardCFFunctionCount
+0x080 GuardCFFunctionTable : Uint8B
+0x088 GuardCFFunctionCount : Uint8B
0:000> !dh -f xul
File Type: DLL
FILE HEADER VALUES
8664 machine (X64)
A number of sections
5C712F6F time date stamp Sat Feb 23 13:33:03 2019
0 file pointer to symbol table
0 number of symbols
F0 size of optional header
2022 characteristics
Executable
App can handle >2gb addresses
DLL
OPTIONAL HEADER VALUES
20B magic #
14.00 linker version
498B200 size of code
195B400 size of initialized data
0 size of uninitialized data
4987AB0 address of entry point
1000 base of code
----- new -----
0000000180000000 image base
1000 section alignment
200 file alignment
2 subsystem (Windows GUI)
6.01 operating system version
0.00 image version
6.01 subsystem version
63CB000 size of image
400 size of headers
62F31FE checksum
0000000000100000 size of stack reserve
0000000000001000 size of stack commit
0000000000100000 size of heap reserve
0000000000001000 size of heap commit
4160 DLL characteristics
High entropy VA supported
Dynamic base
NX compatible
Guard
545F647 [ B6B] address [size] of Export Directory
54601B2 [ 398] address [size] of Import Directory
632A000 [ 24D0] address [size] of Resource Directory
55B7000 [ 204858] address [size] of Exception Directory
62E6C00 [ 1DD0] address [size] of Security Directory
632D000 [ 9D954] address [size] of Base Relocation Directory
54086A2 [ 1C] address [size] of Debug Directory
0 [ 0] address [size] of Description Directory
0 [ 0] address [size] of Special Directory
51602C0 [ 28] address [size] of Thread Storage Directory
4AEC160 [ 100] address [size] of Load Configuration Directory
0 [ 0] address [size] of Bound Import Directory
5464608 [ 40B8] address [size] of Import Address Table Directory
545EE58 [ 160] address [size] of Delay Import Directory
0 [ 0] address [size] of COR20 Header Directory
0 [ 0] address [size] of Reserved Directory
0:000> dps xul+4AEC160+80 L2
00000001`84aec1e0 00000001`85408717 xul!__guard_fids_table
00000001`84aec1e8 00000000`000158e0
0:000> dd xul!__guard_fids_table L8
00000001`85408717 000029b0 00002ab0 00002b10 00002b80
00000001`85408727 00002bc0 000056e0 00006c80 00006cb0
0:000> .foreach /pS 1 /ps 1 (addr { dd /c1 xul!__guard_fids_table L8 } ) { .printf "%y\n", xul+${addr} }
xul!XRE_GetBootstrap (00000001`800029b0)
xul!mozilla::BootstrapImpl::NS_LogInit (00000001`80002ab0)
xul!mozilla::BootstrapImpl::XRE_StartupTimelineRecord (00000001`80002b10)
xul!mozilla::BootstrapImpl::XRE_EnableSameExecutableForContentProc (00000001`80002b80)
xul!mozilla::BootstrapImpl::XRE_main (00000001`80002bc0)
xul!std::_Func_impl_no_alloc<`lambda at z:/task_1550916350/build/src/xpcom/base/Logging.cpp:197:34',void,const nsTSubstring<char> &>::_Delete_this (00000001`800056e0)
xul!nsTHashtable<nsBaseHashtableET<nsCharPtrHashKey,nsAutoPtr<mozilla::LogModule> > >::s_HashKey (00000001`80006c80)
xul!nsTHashtable<nsBaseHashtableET<nsCharPtrHashKey,nsAutoPtr<mozilla::LogModule> > >::s_InitEntry (00000001`80006cb0)
You'd want to use 0x158e0 (or the correct size for another module) instead of 8 in the last command, obviously.
(In reply to temp.stuff from comment #5)
dumpbindoesn't load PDBs for the modules it dumps AFAIK and therefore
doesn't know about most symbols. The few it did display are probably
exported.
I see way more entries in dumpbin's list than there are exports, so there must be something going on, but in any case, thanks very much for the tip!
It's a bit tricky because in general arm64 has way more entries than x86_64, but while looking in a diff viewer at only the lines that are missing on arm64, one thing stands out: a lot of Rust functions are getting left out.
$ grep -c std::io *
arm64.txt:0
x86_64.txt:91
$ grep -c style:: *
arm64.txt:1
x86_64.txt:1955
$ grep -c core:: *
arm64.txt:1
x86_64.txt:1308
$ grep -c webrender:: *
arm64.txt:0
x86_64.txt:665
$ grep -c cranelift *
arm64.txt:0
x86_64.txt:620
$ grep -c baldrdash:: *
arm64.txt:0
x86_64.txt:23
$ grep -c alloc:: *
arm64.txt:0
x86_64.txt:492
It's not 100% broken though: notice that 1 result for core and stylo.
core::ptr::write<style::values::generics::counters::CounterPair<i32>> is in the CFG table.
On arm64, rustc was not marking these functions as IMAGE_SYM_DTYPE_FUNCTION, so we didn't get past the test in lld's maybeAddAddressTakenFunction.
This sounds exactly like https://reviews.llvm.org/D53540#1326473, which was fixed in LLVM r348875.
The fix ought to be in nightly rustc already, at least as far back as https://github.com/rust-lang/rust/commit/df0466d0bb807a7266cc8ac9931cd43b3e84b62e (16 January).
|
|
||
Comment 9•5 years ago
|
||
I see way more entries in dumpbin's list than there are exports, so there must be something going on, but in any case, thanks very much for the tip!
That's why I said probably exported. :) It just wasn't important enough to dig into given that a solution was available.
(In reply to David Major [:dmajor] from comment #8)
On arm64, rustc was not marking these functions as
IMAGE_SYM_DTYPE_FUNCTION, so we didn't get past the test in lld'smaybeAddAddressTakenFunction.This sounds exactly like https://reviews.llvm.org/D53540#1326473, which was fixed in LLVM r348875.
The fix ought to be in nightly rustc already, at least as far back as https://github.com/rust-lang/rust/commit/df0466d0bb807a7266cc8ac9931cd43b3e84b62e (16 January).
Great job. I'm looking forward to a new nightly of Firefox using a new version of Rust with CFG on ARM64 back on, even if it takes some time.
|
Assignee | |
Comment 10•5 years ago
|
||
In addition to an updated Rust, we will also need to pick up https://bugs.llvm.org/show_bug.cgi?id=39799 / https://reviews.llvm.org/rLLD355141 for our lld.
|
|
Assignee | |
Comment 11•5 years ago
|
||
Cherry-picks https://bugs.llvm.org/show_bug.cgi?id=39799.
|
||
Comment 12•5 years ago
|
||
Pushed by dmajor@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/98013639d600 Pick up LLVM fix for CFG on arm64-windows builds r=froydnj
|
||
Comment 13•5 years ago
|
||
Backed out 2 changesets (bug 1523526, bug 1526443) for Be bustage on Windows AArch
Backout: https://hg.mozilla.org/integration/autoland/rev/8fe96c8d21b7a9fdebdf3300de2367f2cc9232c3
Failure push: uhttps://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&group_state=expanded&revision=98013639d60026eb3a0344fa499a321aec176d2b
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247379721&repo=autoland&lineNumber=930
16:52:09 INFO - MOZILLA_OFFICIAL=1
16:52:09 INFO - CFLAGS= -fcrash-diagnostics-dir=z:/task_1558369429/public/build
16:52:09 INFO - WIN32_REDIST_DIR=z:/task_1558369429/build/src/vs2017_15.9.6/VC/redist/arm64/Microsoft.VC141.CRT
16:52:09 INFO - VSPATH=/z/task_1558369429/build/src/vs2017_15.9.6
16:52:09 INFO - TOOLTOOL_DIR=z:/task_1558369429/build/src
16:52:09 INFO - MOZ_REQUIRE_SIGNING=0
16:52:09 INFO - DIAGNOSTICS_DIR=/z/task_1558369429/public/build
16:52:09 INFO - VSWINPATH=z:/task_1558369429/build/src/vs2017_15.9.6
16:52:09 INFO - NO_CACHE=1
16:52:09 INFO - MOZ_AUTOMATION_ARTIFACT_BUILDS=1
16:52:09 INFO - checking for vcs source checkout... hg
16:52:09 INFO - checking for a shell... C:/mozilla-build/msys/bin/sh.exe
16:52:09 INFO - checking for host system type... x86_64-pc-mingw32
16:52:09 INFO - checking for target system type... aarch64-windows-mingw32
16:52:09 INFO - checking whether cross compiling... yes
16:52:09 ERROR - Traceback (most recent call last):
16:52:09 INFO - File "z:/task_1558369429/build/src/configure.py", line 132, in <module>
16:52:09 INFO - sys.exit(main(sys.argv))
16:52:09 INFO - File "z:/task_1558369429/build/src/configure.py", line 38, in main
16:52:09 INFO - sandbox.run(os.path.join(os.path.dirname(file), 'moz.configure'))
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init_.py", line 441, in run
16:52:09 INFO - self.value_for(option)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init.py", line 528, in _value_for
16:52:09 INFO - return self.value_for_option(obj)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\util.py", line 947, in method_call
16:52:09 INFO - cache[args] = self.func(instance, *args)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init.py", line 591, in _value_for_option
16:52:09 INFO - % option_string.split('=', 1)[0])
16:52:09 INFO - mozbuild.configure.options.InvalidOptionError: --enable-hardening is not available in this configuration
16:52:09 INFO - *** Fix above errors and then restart with
16:52:09 INFO - "./mach build"
16:52:09 INFO - client.mk:111: recipe for target 'configure' failed
16:52:09 INFO - mozmake.EXE: *** [configure] Error 1
16:52:10 ERROR - Return code: 2
16:52:10 WARNING - setting return code to 2
16:52:10 FATAL - 'mach build -v' did not run successfully. Please check log for errors.
16:52:10 FATAL - Running post_fatal callback...
16:52:10 FATAL - Exiting -1
16:52:10 INFO - [mozharness: 2019-05-20 16:52:10.017000Z] Finished build step (failed)
16:52:10 INFO - Running post-run listener: _parse_build_tests_ccov
16:52:10 INFO - Running post-run listener: _shutdown_sccache
16:52:10 INFO - Running post-run listener: _summarize
16:52:10 ERROR - # TBPL FAILURE #
16:52:10 INFO - [mozharness: 2019-05-20 16:52:10.018000Z] FxDesktopBuild summary:
16:52:10 ERROR - # TBPL FAILURE #
|
|
||
Comment 14•5 years ago
|
||
Pushed by dmajor@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c34880f0f630 Pick up LLVM fix for CFG on arm64-windows builds r=froydnj
|
||
Comment 15•5 years ago
|
||
| bugherder | ||
Description
•