Add unit test to check for broker FD leak

sandbox/linux/services/broker_process_unittest.cc

 // Copyright (c) 2012 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 "sandbox/linux/services/broker_process.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/posix/eintr_wrapper.h"
+#include "base/posix/unix_domain_socket_linux.h"
 #include "sandbox/linux/tests/test_utils.h"
 #include "sandbox/linux/tests/unit_tests.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace sandbox {
 
+class BrokerProcessTestHelper {
+ public:
+  static int get_ipc_socketpair(const BrokerProcess* broker) {
+    return broker->ipc_socketpair_;
+  }
+};
+
 namespace {
 
 // Creates and open a temporary file on creation and closes
 // and removes it on destruction.
 // Unlike base/ helpers, this does not require JNI on Android.
 class ScopedTemporaryFile {
  public:
   ScopedTemporaryFile()
       : fd_(-1) {
 #if defined(OS_ANDROID)
@@ skipping 387 matching lines @@
   // Don't do anything here, so that ASSERT works in the subfunction as
   // expected.
 }
 
 TEST(BrokerProcess, OpenComplexFlagsNoClientCheck) {
   TestOpenComplexFlags(false /* fast_check_in_client */);
   // Don't do anything here, so that ASSERT works in the subfunction as
   // expected.
 }
 
+// We need to allow noise because the broker will log when it receives our
+// bogus IPCs.
+SANDBOX_TEST_ALLOW_NOISE(BrokerProcess, RecvMsgDescriptorLeak) {
+  // Find the four lowest available file descriptors.

[jln (very slow on Chromium), 2014/04/10 20:21:15]

Replace with "Find four available file descriptors" instead? They're not
guaranteed to be the lowest.

[mdempsky, 2014/04/10 20:35:51]

As discussed, POSIX requires pipe() to return the two lowest file descriptors
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/pipe.html), so I left
the comment as is.

+  int available_fds[4];
+  SANDBOX_ASSERT(0 == pipe(available_fds));
+  SANDBOX_ASSERT(0 == pipe(available_fds + 2));
+
+  // Save one FD to send to the broker later, and close the others.
+  for (size_t i = 1; i < arraysize(available_fds); i++) {

[jln (very slow on Chromium), 2014/04/10 20:21:15]

Didn't you forget to close available_fds[0]?

Make it a ScopedFD maybe?

[mdempsky, 2014/04/10 20:35:51]

Done.

+    SANDBOX_ASSERT(0 == IGNORE_EINTR(close(available_fds[i])));
+  }
+
+  // Lower our file descriptor limit to just allow three more file descriptors
+  // to be allocated.  (N.B., RLIMIT_NOFILE doesn't limit the number of file
+  // descriptors a process can have: it only limits the highest value that can
+  // be assigned to newly-created descriptors allocated by the process.)
+  const rlim_t fd_limit =

[jln (very slow on Chromium), 2014/04/10 20:21:15]

kFdLimit

[mdempsky, 2014/04/10 20:35:51]

As discussed, this isn't a "compile-time constant", so I think kNaming it would
be misleading.

+      1 + *std::max_element(available_fds,
+                            available_fds + arraysize(available_fds));
+  const struct rlimit new_rlim = {fd_limit, fd_limit};
+  SANDBOX_ASSERT(0 == setrlimit(RLIMIT_NOFILE, &new_rlim));
+
+  const char kCpuInfo[] = "/proc/cpuinfo";
+  std::vector<std::string> read_whitelist;
+  read_whitelist.push_back(kCpuInfo);
+
+  BrokerProcess open_broker(EPERM, read_whitelist, std::vector<std::string>());
+  SANDBOX_ASSERT(open_broker.Init(base::Bind(&NoOpCallback)));
+
+  const int ipc_fd = BrokerProcessTestHelper::get_ipc_socketpair(&open_broker);
+  SANDBOX_ASSERT(ipc_fd >= 0);
+
+  static const char kBogus[] = "not a pickle";
+  std::vector<int> fds;
+  fds.push_back(available_fds[0]);
+
+  // The broker process should only have a couple spare file descriptors

[jln (very slow on Chromium), 2014/04/10 20:21:15]

Sending kFdLimit descriptors is guaranteed to work, but I don't think you can
guarantee that only a couple spare fd are available.

[mdempsky, 2014/04/10 20:35:51]

Yep, that's what I was trying to convey with the comment: because of the earlier
setrlimit logic, the broker "should" only have a couple spare fds, but instead
of relying on that I just guarantee that we'd flood the descriptor table.  Let
me know if you think I should revise the comment.

+  // available, but for good measure we send it fd_limit bogus IPCs anyway.
+  for (rlim_t i = 0; i < fd_limit; ++i) {
+    SANDBOX_ASSERT(
+        UnixDomainSocket::SendMsg(ipc_fd, kBogus, sizeof(kBogus), fds));
+  }
+
+  const int fd = open_broker.Open(kCpuInfo, O_RDONLY);
+  SANDBOX_ASSERT(fd >= 0);
+  SANDBOX_ASSERT(0 == IGNORE_EINTR(close(fd)));
+}
+
 }  // namespace sandbox