Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(797)

Unified Diff: sandbox/win/src/target_services.cc

Issue 2726733003: CSRSS lockdown: destroy CSRSS heap (Closed)
Patch Set: refactor heap code to heap_helper, add some explicit tests of these heap_helper functions Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« sandbox/win/src/lpc_policy_test.cc ('K') | « sandbox/win/src/lpc_policy_test.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sandbox/win/src/target_services.cc
diff --git a/sandbox/win/src/target_services.cc b/sandbox/win/src/target_services.cc
index ec2fa7134c8b46663a809ff09c38dc9b517bf2e4..1a8228c4f8d79d359cc2487cde8b2f79a2bb9fd3 100644
--- a/sandbox/win/src/target_services.cc
+++ b/sandbox/win/src/target_services.cc
@@ -12,6 +12,7 @@
#include "base/win/windows_version.h"
#include "sandbox/win/src/crosscall_client.h"
#include "sandbox/win/src/handle_closer_agent.h"
+#include "sandbox/win/src/heap_helper.h"
#include "sandbox/win/src/ipc_tags.h"
#include "sandbox/win/src/process_mitigations.h"
#include "sandbox/win/src/restricted_token_utils.h"
@@ -45,16 +46,34 @@ bool FlushCachedRegHandles() {
FlushRegKey(HKEY_USERS));
}
+// Cleans up this process if CSRSS will be disconnected, as this disconnection
+// is not supported Windows behavior.
+// Currently, this step requires closing a heap that this shared with csrss.exe.
+// Closing the ALPC Port handle to csrss.exe leaves this heap in an invalid
+// state. This causes problems if anyone enumerates the heap.
+bool CsrssDisconnectCleanup() {
+ PVOID csr_port_heap = sandbox::FindCsrPortHeap();
Will Harris 2017/03/22 19:21:49 are we not already in sandbox namespace. hmm? stra
Will Harris 2017/03/22 19:21:49 implicit cast from HANDLE to PVOID - is this what
liamjm (20p) 2017/04/14 17:27:20 No. changed to HANDLE. Thanks.
liamjm (20p) 2017/04/14 17:27:20 Yeah... Just in an unnamed namespace at this poin
Will Harris 2017/05/01 18:33:15 yes it seems to make sense to move all these funct
+ if (nullptr == csr_port_heap) {
Will Harris 2017/03/22 19:21:49 !csr_port_heap
liamjm (20p) 2017/04/14 17:27:20 Done.
+ LOG(ERROR) << "Failed to find CSR Port heap handle" return false;
Will Harris 2017/03/22 19:21:49 win\src\target_services.cc(57): error C2143: synta
liamjm (20p) 2017/04/14 17:27:20 Done.
+ }
+ HeapDestroy(csr_port_heap);
+ return true;
+}
+
// Checks if we have handle entries pending and runs the closer.
// Updates is_csrss_connected based on which handle types are closed.
bool CloseOpenHandles(bool* is_csrss_connected) {
if (sandbox::HandleCloserAgent::NeedsHandlesClosed()) {
sandbox::HandleCloserAgent handle_closer;
handle_closer.InitializeHandlesToClose(is_csrss_connected);
+ if (!*is_csrss_connected) {
+ if (!CsrssDisconnectCleanup()) {
+ return false;
+ }
+ }
if (!handle_closer.CloseHandles())
return false;
}
-
return true;
}
« sandbox/win/src/lpc_policy_test.cc ('K') | « sandbox/win/src/lpc_policy_test.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698