Chrome: change stack canaries on fork [chromium/src : main]
Matthew Denton (Gerrit)
Attention is currently required from: Avi Drissman.
1 comment:
Patchset:
Avi PTAL at this followup
To view, visit change 3040832. To unsubscribe, or for help writing mail filters, visit settings.
Avi Drissman (Gerrit)
Attention is currently required from: Matthew Denton.
Patch set 3:Code-Review +1
1 comment:
File chrome/browser/chrome_content_browser_client.cc:
// Opt into a hardened stack canary mitigation if it hasn't already been
// force-disabled.If this only has an effect on Linux/ChromeOS, do we want to move it into an #if block (like the one above) to make that obvious?
To view, visit change 3040832. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Denton (Gerrit)
2 comments:
Patchset:
Thanks!
File chrome/browser/chrome_content_browser_client.cc:
// Opt into a hardened stack canary mitigation if it hasn't already been
// force-disabled.
If this only has an effect on Linux/ChromeOS, do we want to move it into an #if block (like the one […]
Done
To view, visit change 3040832. To unsubscribe, or for help writing mail filters, visit settings.
Avi Drissman (Gerrit)
Attention is currently required from: Matthew Denton.
Patch set 4:Code-Review +1
1 comment:
Patchset:
SLG
To view, visit change 3040832. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Denton (Gerrit)
Attention is currently required from: Matthew Denton.
Patch set 4:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
Chrome: change stack canaries on fork
Adds the command line switch to Chrome that changes the stack canary
on fork, which strengthens the stack canary security mitigation, which
can currently be brute forced.
This command line switch can be force disabled manually on the browser
command line. This is done so in PrepareBrowserCommandLineForTests()
as changing the stack canary will cause crashes when the browser
terminates, and we don't need hardened security mitigations for all of
our tests.
This only has an effect on Linux and ChromeOS where we fork.
Bug: 1206626
Change-Id: I68e1e0322ac5d14be5fc7626c750fe5dc5c9fc3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3040832
Reviewed-by: Avi Drissman <a...@chromium.org>
Commit-Queue: Matthew Denton <mpde...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905207}
---
M chrome/browser/chrome_content_browser_client.cc
M chrome/test/base/test_launcher_utils.cc
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 8913e0aa..78e1aa4 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -2364,7 +2364,14 @@
command_line->HasSwitch(sandbox::policy::switches::kNoSandbox)) {
command_line->AppendSwitch(switches::kEnableThreadInstructionCount);
}
-#endif
+
+ // Opt into a hardened stack canary mitigation if it hasn't already been
+ // force-disabled.
+ if (!browser_command_line.HasSwitch(switches::kChangeStackGuardOnFork)) {
+ command_line->AppendSwitchASCII(switches::kChangeStackGuardOnFork,
+ switches::kChangeStackGuardOnForkEnabled);
+ }
+#endif // defined(OS_LINUX) || defined(OS_CHROMEOS)
}
std::string
diff --git a/chrome/test/base/test_launcher_utils.cc b/chrome/test/base/test_launcher_utils.cc
index f796624d..2b11659 100644
--- a/chrome/test/base/test_launcher_utils.cc
+++ b/chrome/test/base/test_launcher_utils.cc
@@ -70,6 +70,15 @@
#endif
command_line->AppendSwitch(switches::kDisableComponentUpdate);
+
+#if defined(OS_LINUX) || defined(OS_CHROMEOS)
+ // Changing the stack canary means we need to disable the stack guard on all
+ // functions that appear as ancestors in the call stack of RunZygote(). This
+ // is infeasible for tests, and changing the stack canary is unnecessary for
+ // tests as it is a security mitigation.
+ command_line->AppendSwitchASCII(switches::kChangeStackGuardOnFork,
+ switches::kChangeStackGuardOnForkDisabled);
+#endif
}
void PrepareBrowserCommandLineForBrowserTests(base::CommandLine* command_line,
To view, visit change 3040832. To unsubscribe, or for help writing mail filters, visit settings.