platform_ToolchainOptions failing on Chrome PFQ |
||||||||
Issue description
For the last two runs
06/25 02:53:27.499 INFO | base_sysinfo:0395| ChromeOS BOARD = peppy_1.7GHz_4GB
06/25 02:56:29.188 ERROR|platform_Toolchain:0321| Test LOAD Writable and Exec 2 failures
FAILED:
/opt/google/chrome/libmojo_core_arc32.so
/opt/google/chrome/libmojo_core_arc64.so
06/25 02:56:29.219 WARNI| test:0637| The test failed with the following exception
Traceback (most recent call last):
File "/usr/local/autotest/common_lib/test.py", line 631, in _exec
_call_test_function(self.execute, *p_args, **p_dargs)
File "/usr/local/autotest/common_lib/test.py", line 831, in _call_test_function
return func(*args, **dargs)
File "/usr/local/autotest/common_lib/test.py", line 495, in execute
dargs)
File "/usr/local/autotest/common_lib/test.py", line 362, in _call_run_once_with_retry
postprocess_profiled_run, args, dargs)
File "/usr/local/autotest/common_lib/test.py", line 400, in _call_run_once
self.run_once(*args, **dargs)
File "/usr/local/autotest/tests/platform_ToolchainOptions/platform_ToolchainOptions.py", line 324, in run_once
raise error.TestFail(fail_summary_msg)
TestFail: Test LOAD Writable and Exec 2 failures
,
Jun 25 2018
+jorgelo who might know better than me on what this test is testing against I am guessing libmojo from chrome has added something different.
,
Jun 25 2018
https://uberchromegw.corp.google.com/i/chromeos/waterfall?show=master-chromium-pfq amd64-generic and betty doesn't fail, but at least: https://luci-milo.appspot.com/buildbot/chromeos/peach_pit-chrome-pfq/5833 https://luci-milo.appspot.com/buildbot/chromeos/tricky-chrome-pfq/5654 https://luci-milo.appspot.com/buildbot/chromeos/peppy-chrome-pfq/5826 fails
,
Jun 25 2018
This probably needs a better error message. This test is testing that we're enabling compiler-based hardening measures for our binaries. It looks like libmojo_core has writable and executable LOAD program headers, which means that libmojo will have writable and executable memory mappings, by default, which facilitates exploitation. How is libmojo_core being compiled?
,
Jun 25 2018
It's a new target, and it's being built from Chrome: https://cs.chromium.org/chromium/src/mojo/edk/BUILD.gn?q=libmojo_core_arc32&sq=package:chromium&g=0&l=234
,
Jun 25 2018
Seems https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1106281 caused it? The pfq started to fail at Jun 21st, so the time matches. Besides, it seems broke PFQ before with a different failure, see Issue 846393 . rockot@, could you confirm?
,
Jun 25 2018
Ah, great catch, the binaries are being built with a different toolchain! That explains. And it also explains the necessity of this test =). Ken, why do we need to use the NDK for this? Is it because these .so's live inside the container?
,
Jun 25 2018
These SOs are to be used by things running inside the container, yeah.
,
Jun 25 2018
Well, we gotta figure out a way to compile them without W+X sections.
,
Jun 25 2018
Will revert this CL first.
,
Jun 25 2018
jorgelo@ can you please elaborate on how I can verify the presence of this W+X section, and how we prevent other binaries from having one? There isn't really anything special about this library, it's a normal shared library target in the Chrome tree using the same toolchain we use to build everything else for Android.
,
Jun 25 2018
,
Jun 25 2018
The test checks by using readelf:
readelf -lW {} 2>&1 | grep \"LOAD\" | egrep -v \"(RW |R E)\"
Kees, what compiler options do we need to prevent W+X LOAD sections?
,
Jun 25 2018
The most common source of W+X is an executable stack segment. This is usually triggered by unannotated raw assembly sections. The "best" solution is to adjust the source code to avoid it. I have a write-up at Ubuntu on it: https://wiki.ubuntu.com/SecurityTeam/Roadmap/ExecutableStacks
,
Jun 25 2018
To big-hammer it, build with "-Wa,--noexecstack" which changes the default marking to non-exec.
,
Jun 25 2018
If the flag mentioned in Comment #15 fixes it, then we need to see if we can add it by default to this toolchain.
,
Jun 25 2018
The CL has been reverted at https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1113957.
,
Jun 25 2018
Well this is confusing. When I build the libraries (for either Intel or arm), I always see three LOAD sections: LOAD 0x000000 0x00000000 0x00000000 0x1b500 0x1b500 R 0x1000 LOAD 0x01c000 0x0001c000 0x0001c000 0x56d60 0x56d60 R E 0x1000 LOAD 0x073000 0x00073000 0x00073000 0x04128 0x08564 RW 0x1000 Is it possible (based on the command line in comment #13) that the test is actually tripping over the read-only section?
,
Jun 25 2018
After looking at the script to confirm, that does appear to be the case. Is there some reason why it shouldn't be filtering out "(RW |R E|R )" instead?
,
Jun 25 2018
Wow, well that's cool. Yeah, that's a bug in the script. And it's the first I've ever seen an entirely read-only LOAD section. :) I assume this must be from recent attempts to split executable and readable sections for the future of X^R?
,
Jun 25 2018
It seems reverting the CL doesn't fix the failure. It still can be seen on the latest informational pfq, see https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8942722291758484992 06/25 13:57:32.909 ERROR|platform_Toolchain:0321| Test LOAD Writable and Exec 2 failures FAILED: /opt/google/chrome/libmojo_core_arc32.so /opt/google/chrome/libmojo_core_arc64.so Since the CL is a Chrome OS change, it should be automatically picked up by the informational PFQ build. Any thought why it might happen?
,
Jun 25 2018
No idea about any of that, but FWIW https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/1114198 should definitely resolve the issue once it lands.
,
Jun 25 2018
Ah. I think I know the reason: the Auto roll chromite change https://chromium-review.googlesource.com/c/chromium/src/+/1113824 also needs to be picked up by Tot chrome for the failure to go away. rockot@, thanks for the quick fix!
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/ef9faf4553d6f88dc397bdc757881970ec09e218 commit ef9faf4553d6f88dc397bdc757881970ec09e218 Author: Ken Rockot <rockot@chromium.org> Date: Tue Jun 26 09:55:46 2018 platform_ToolchainOptions: Allow read-only LOAD sections Fixes the check for W+X LOAD sections so that it no longer falsely rejects ELF binaries with a read-only LOAD section. Bug: 856144 Change-Id: I9a57cff9d24453fe630f35460af2e1e830da3796 Reviewed-on: https://chromium-review.googlesource.com/1114198 Commit-Ready: Ken Rockot <rockot@chromium.org> Tested-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Manoj Gupta <manojgupta@chromium.org> Reviewed-by: Kees Cook <keescook@chromium.org> [modify] https://crrev.com/ef9faf4553d6f88dc397bdc757881970ec09e218/client/site_tests/platform_ToolchainOptions/platform_ToolchainOptions.py
,
Jun 26 2018
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/34874a62555d7bf8fea30814d8b01b56a1dc07b0 commit 34874a62555d7bf8fea30814d8b01b56a1dc07b0 Author: Jorge Lucangeli Obes <jorgelo@chromium.org> Date: Wed Jun 27 00:02:50 2018 platform_ToolchainOptions: Fix comment. BUG= chromium:856144 TEST=precq passes Change-Id: Icaa4ae567a97465620df4bf3e6f08beefd38c3b4 Reviewed-on: https://chromium-review.googlesource.com/1114781 Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> [modify] https://crrev.com/34874a62555d7bf8fea30814d8b01b56a1dc07b0/client/site_tests/platform_ToolchainOptions/platform_ToolchainOptions.py |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by uekawa@google.com
, Jun 25 2018