dbus: revamp fd passing for i/o restrictions

dbus/file_descriptor.cc

Index: dbus/file_descriptor.cc
diff --git a/dbus/file_descriptor.cc b/dbus/file_descriptor.cc
index fc240f552c634524fb619e52095305fd054c5807..2f22d60fa15edd7ad2a1ebe370aaef74c131ea64 100644
--- a/dbus/file_descriptor.cc
+++ b/dbus/file_descriptor.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/logging.h"
 #include "base/platform_file.h"
 #include "dbus/file_descriptor.h"
 
@@ -12,4 +13,26 @@ FileDescriptor::~FileDescriptor() {
     base::ClosePlatformFile(value_);
 }
 
+// Retrieves value as an int without affecting ownership.

[satorux1, 2012/05/10 07:16:56]

the comment looks identical to the one in .h, if so, please remove.

[Sam Leffler, 2012/05/10 15:59:32]

Done.

+int FileDescriptor::value() const {
+  CHECK(valid_);

[satorux1, 2012/05/09 22:12:09]

We usually use DCHECK(). IIRC, CHECK is used only when we want to catch a crash
in shipping binaries, which we rarely do.

[satorux1, 2012/05/10 07:16:56]

I think I was wrong. If valid_ is false, this is security violation, so so
crashing the browser by CHECK() would make more sense than DCHECK.

[Sam Leffler, 2012/05/10 15:59:32]

Correct, this preserves the previous behaviour that generated an assert check in
release binaries due to potential security violation.

+  return value_;
+}
+
+// Takes the value and ownership.

[satorux1, 2012/05/10 07:16:56]

the comment looks identical to the one in .h, if so, please remove.

[Sam Leffler, 2012/05/10 15:59:32]

Done.

+int FileDescriptor::TakeValue() {
+  CHECK(valid_);  // NB: check first so owner_ is unchanged if this triggers

[satorux1, 2012/05/09 22:12:09]

ditto.

+  owner_ = false;
+  return value_;
+}
+
+// Checks if |value_| is suitable for sending/receiving.  We disallow
+// directories to avoid potential sandbox escapes.  Note this call must
+// be made on the File thread.

[satorux1, 2012/05/10 07:16:56]

move this comment to .h file? The users of this function want to know about the
information.

on the File thread -> something like "on a thread with file IO allowed"

[Sam Leffler, 2012/05/10 15:59:32]

Done.

+void FileDescriptor::CheckValidity() {
+  base::PlatformFileInfo info;
+  bool ok = base::GetPlatformFileInfo(value_, &info);
+  valid_ = (ok && !info.is_directory);
+}
+
 }  // namespace dbus