Index: base/files/protect_file_posix.cc |
diff --git a/base/files/protect_file_posix.cc b/base/files/protect_file_posix.cc |
new file mode 100644 |
index 0000000000000000000000000000000000000000..d06ff0576e3423ad39f5109f46d7d1dc7f8bbb41 |
--- /dev/null |
+++ b/base/files/protect_file_posix.cc |
@@ -0,0 +1,99 @@ |
+// Copyright 2014 The Chromium Authors. All rights reserved. |
+// Use of this source code is governed by a BSD-style license that can be |
+// found in the LICENSE file. |
+ |
+#include "base/containers/hash_tables.h" |
+#include "base/files/file.h" |
+#include "base/lazy_instance.h" |
+#include "base/logging.h" |
+#include "base/posix/eintr_wrapper.h" |
+#include "base/synchronization/lock.h" |
+ |
+// These hooks provided for base::File perform additional sanity checks when |
+// files are closed. |
+// |
+// Background: |
+// 1. The browser process crashes if a call to close() provided by the C |
+// library (i.e. close(3)) fails. This is done for security purposes. See |
+// base/files/scoped_file.cc. When a crash like this happens, there is not |
+// enough context in the minidump to triage the problem. |
+// 2. base::File provides a good abstraction to prevent closing incorrect |
+// file descriptors or double-closing. Closing non-owned file descriptors |
+// would more likely happen from outside base::File and base::ScopedFD. |
+// |
+// These hooks intercept base::File operations to 'protect' file handles / |
+// descriptors from accidental close(3) by other portions of the code being |
+// linked into the browser. Also, this file provides an interceptor for the |
+// close(3) itself, and requires to be linked with cooperation of |
+// --Wl,--wrap=close (i.e. linker wrapping). |
+// |
+// Wrapping close(3) on all libraries can lead to confusion, particularly for |
+// the libraries that do not use ::base (I am also looking at you, |
+// crazy_linker). Instead two hooks are added to base::File, which are |
gavinp
2014/11/04 15:02:02
Nit: one space after .
pasko
2014/11/04 17:48:06
Done.
|
+// implemented as no-op by default. This file should be linked into the chrome |
gavinp
2014/11/04 15:02:02
Nit: capitalise Chrome.
pasko
2014/11/04 17:48:06
Done.
|
+// native binary(-ies) for a whitelist of targets where "file descriptor |
+// protection" is useful. |
+ |
+// With compilers other than GCC the wrapper is trivial. This is to avoid |
Fabrice (no longer in Chrome)
2014/11/04 15:37:21
Nit: GCC and Clang
pasko
2014/11/04 17:48:06
Done.
|
+// overexercising mechanisms for overriding weak symbols. |
+#if !defined(COMPILER_GCC) |
+extern "C" { |
+ |
+int __real_close(int fd); |
+ |
+BASE_EXPORT int __wrap_close(int fd) { |
+ return __real_close(fd); |
+} |
+ |
+} // extern "C" |
+ |
+#else // defined(COMPILER_GCC) |
+ |
+namespace { |
+ |
+// Protects the |g_protected_fd_set|. |
+base::LazyInstance<base::Lock>::Leaky g_lock = |
+ LAZY_INSTANCE_INITIALIZER; |
gavinp
2014/11/04 15:02:02
Nit: can this go up on the previous line?
pasko
2014/11/04 17:48:06
Done.
|
+ |
+// Holds the set of all 'protected' file descriptors. |
+typedef base::hash_set<int> IntSet; |
gavinp
2014/11/04 15:02:01
Do we need this typedef? It's only used once, so i
pasko
2014/11/04 17:48:06
Done.
|
+base::LazyInstance<IntSet>::Leaky g_protected_fd_set = |
+ LAZY_INSTANCE_INITIALIZER; |
+ |
+bool IsFileDescriptorProtected(int fd) { |
+ base::AutoLock lock(*g_lock.Pointer()); |
+ return g_protected_fd_set.Get().count(fd) != 0; |
+} |
+ |
+} // namespace |
+ |
+namespace base { |
+ |
+BASE_EXPORT void ProtectFileDescriptor(int fd) { |
+ base::AutoLock lock(g_lock.Get()); |
gavinp
2014/11/04 15:02:02
Sad about the lock.
pasko
2014/11/04 17:48:06
Acknowledged. Agreed. Let's get drunk.
|
+ g_protected_fd_set.Get().insert(fd); |
gavinp
2014/11/04 15:02:01
Is double protecting also a crash?
pasko
2014/11/04 17:48:06
Yes. Thanks. Done.
|
+} |
+ |
+BASE_EXPORT void UnprotectFileDescriptor(int fd) { |
+ base::AutoLock lock(*g_lock.Pointer()); |
+ g_protected_fd_set.Get().erase(fd); |
gavinp
2014/11/04 15:02:02
Is double erasing also a crash?
pasko
2014/11/04 17:48:06
Yes! Done.
|
+} |
+ |
+} // namespace base |
+ |
+extern "C" { |
+ |
+int __real_close(int fd); |
+ |
+BASE_EXPORT int __wrap_close(int fd) { |
+ // It is important to crash here and provide a minidump with necessary context |
gavinp
2014/11/04 15:02:01
This comment could be a bit more explanatory. Ther
pasko
2014/11/04 17:48:06
Sorry, it was indeed a bit too short. Rephrased cl
|
+ // about the call to close() that was done outside base::File, but on a FD |
+ // belonging to a base::File. |
+ CHECK(!IsFileDescriptorProtected(fd)) << "fd: " << fd; |
+ PCHECK(0 == IGNORE_EINTR(__real_close(fd))); |
gavinp
2014/11/04 15:02:01
PCHECK_EQ?
I believe close can fail with EBADF (l
pasko
2014/11/04 17:48:06
I blindly copied what's in base/files/scoped_file.
|
+ return 0; |
+} |
+ |
+} // extern "C" |
+ |
+#endif // defined(COMPILER_GCC) |