[Fuchsia] Fix leaks in Zircon handle serialization in IPC

This change includes the following fixes in IPC code that works with
Zircon handles:
 - Removed one of the two constructors in HandleAttachmentFuchsia.
   Unlike the other constructor it was not taking ownership of the
   and so it was error-prone.
 - There is only one case when VMO handles need to be duplicated in the
   IPC layer. Now that case is handled directly in
   ParamTraits<base::SharedMemoryHandle>::Write() instead of the removed
   HandleAttachementFuchsia constructor.
 - ParamTraits<base::SharedMemoryHandle>::Write() now correctly handles
   OwnershipPassesToIPC(). Previously those handles were leaked.
 - Fixed ParamTraits<zx::vmo>::Write() not to leak handles.
 - Removed ipc::HandleFuchsia class. It's no longer useful.

Bug: 977435
Change-Id: Ibaa2df136a3753217cbb609fbcb1c76af55ce792
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670568
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671317}

ipc/BUILD.gn

@@ -157,8 +157,6 @@ component("message_support") {
     sources += [
       "handle_attachment_fuchsia.cc",
       "handle_attachment_fuchsia.h",
-      "handle_fuchsia.cc",
-      "handle_fuchsia.h",
     ]
   }
 

ipc/handle_attachment_fuchsia.cc

@@ -12,13 +12,6 @@
 namespace IPC {
 namespace internal {
 
-HandleAttachmentFuchsia::HandleAttachmentFuchsia(zx_handle_t handle) {
-  zx_status_t result =
-      zx::unowned_handle(handle)->duplicate(ZX_RIGHT_SAME_RIGHTS, &handle_);
-  if (result != ZX_OK)
-    ZX_DLOG(ERROR, result) << "zx_handle_duplicate";
-}
-
 HandleAttachmentFuchsia::HandleAttachmentFuchsia(zx::handle handle)
     : handle_(std::move(handle)) {}
 

ipc/handle_attachment_fuchsia.h

@@ -8,7 +8,6 @@
 #include <lib/zx/handle.h>
 #include <stdint.h>
 
-#include "ipc/handle_fuchsia.h"
 #include "ipc/ipc_message_attachment.h"
 #include "ipc/ipc_message_support_export.h"
 
@@ -19,10 +18,6 @@ namespace internal {
 class IPC_MESSAGE_SUPPORT_EXPORT HandleAttachmentFuchsia
     : public MessageAttachment {
  public:
-  // This constructor makes a copy of |handle| and takes ownership of the
-  // result. Should only be called by the sender of a Chrome IPC message.
-  explicit HandleAttachmentFuchsia(zx_handle_t handle);
-
   // This constructor takes ownership of |handle|. Should only be called by the
   // receiver of a Chrome IPC message.
   explicit HandleAttachmentFuchsia(zx::handle handle);

ipc/handle_fuchsia.cc

@@ -1,52 +0,0 @@
-// Copyright 2017 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 "ipc/handle_fuchsia.h"
-
-#include <utility>
-
-#include "base/logging.h"
-#include "base/memory/ref_counted.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/strings/stringprintf.h"
-#include "ipc/handle_attachment_fuchsia.h"
-#include "ipc/ipc_message.h"
-
-namespace IPC {
-
-HandleFuchsia::HandleFuchsia() : handle_(ZX_HANDLE_INVALID) {}
-
-HandleFuchsia::HandleFuchsia(const zx_handle_t& handle) : handle_(handle) {}
-
-// static
-void ParamTraits<HandleFuchsia>::Write(base::Pickle* m, const param_type& p) {
-  scoped_refptr<IPC::internal::HandleAttachmentFuchsia> attachment(
-      new IPC::internal::HandleAttachmentFuchsia(p.get_handle()));
-  if (!m->WriteAttachment(std::move(attachment)))
-    NOTREACHED();
-}
-
-// static
-bool ParamTraits<HandleFuchsia>::Read(const base::Pickle* m,
-                                      base::PickleIterator* iter,
-                                      param_type* r) {
-  scoped_refptr<base::Pickle::Attachment> base_attachment;
-  if (!m->ReadAttachment(iter, &base_attachment))
-    return false;
-  MessageAttachment* attachment =
-      static_cast<MessageAttachment*>(base_attachment.get());
-  if (attachment->GetType() != MessageAttachment::Type::FUCHSIA_HANDLE)
-    return false;
-  IPC::internal::HandleAttachmentFuchsia* handle_attachment =
-      static_cast<IPC::internal::HandleAttachmentFuchsia*>(attachment);
-  r->set_handle(handle_attachment->Take());
-  return true;
-}
-
-// static
-void ParamTraits<HandleFuchsia>::Log(const param_type& p, std::string* l) {
-  l->append(base::StringPrintf("0x%x", p.get_handle()));
-}
-
-}  // namespace IPC

ipc/handle_fuchsia.h

@@ -1,47 +0,0 @@
-// Copyright 2017 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.
-
-#ifndef IPC_HANDLE_FUCHSIA_H_
-#define IPC_HANDLE_FUCHSIA_H_
-
-#include <zircon/types.h>
-
-#include <string>
-
-#include "ipc/ipc_message_support_export.h"
-#include "ipc/ipc_param_traits.h"
-
-namespace base {
-class Pickle;
-class PickleIterator;
-}  // namespace base
-
-namespace IPC {
-
-class IPC_MESSAGE_SUPPORT_EXPORT HandleFuchsia {
- public:
-  // Default constructor makes an invalid zx_handle_t.
-  HandleFuchsia();
-  explicit HandleFuchsia(const zx_handle_t& handle);
-
-  zx_handle_t get_handle() const { return handle_; }
-  void set_handle(zx_handle_t handle) { handle_ = handle; }
-
- private:
-  zx_handle_t handle_;
-};
-
-template <>
-struct IPC_MESSAGE_SUPPORT_EXPORT ParamTraits<HandleFuchsia> {
-  typedef HandleFuchsia param_type;
-  static void Write(base::Pickle* m, const param_type& p);
-  static bool Read(const base::Pickle* m,
-                   base::PickleIterator* iter,
-                   param_type* p);
-  static void Log(const param_type& p, std::string* l);
-};
-
-}  // namespace IPC
-
-#endif  // IPC_HANDLE_FUCHSIA_H_

ipc/ipc_message_utils.cc

@@ -36,8 +36,8 @@
 #endif
 
 #if defined(OS_FUCHSIA)
+#include "base/fuchsia/fuchsia_logging.h"
 #include "ipc/handle_attachment_fuchsia.h"
-#include "ipc/handle_fuchsia.h"
 #endif
 
 #if defined(OS_ANDROID)
@@ -635,7 +635,7 @@ void ParamTraits<zx::vmo>::Write(base::Pickle* m, const param_type& p) {
     return;
 
   if (!m->WriteAttachment(new internal::HandleAttachmentFuchsia(
-          const_cast<param_type&>(p).release()))) {
+          std::move(const_cast<param_type&>(p))))) {
     NOTREACHED();
   }
 }
@@ -758,8 +758,17 @@ void ParamTraits<base::SharedMemoryHandle>::Write(base::Pickle* m,
   HandleWin handle_win(p.GetHandle());
   WriteParam(m, handle_win);
 #elif defined(OS_FUCHSIA)
-  HandleFuchsia handle_fuchsia(p.GetHandle());
-  WriteParam(m, handle_fuchsia);
+  zx::vmo vmo;
+  if (p.OwnershipPassesToIPC()) {
+    vmo = zx::vmo(p.GetHandle());
+  } else {
+    zx_status_t result =
+        zx::unowned_vmo(p.GetHandle())->duplicate(ZX_RIGHT_SAME_RIGHTS, &vmo);
+    if (result != ZX_OK)
+      ZX_DLOG(ERROR, result) << "zx_handle_duplicate";
+  }
+
+  WriteParam(m, vmo);
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   MachPortMac mach_port_mac(p.GetMemoryObject());
   WriteParam(m, mach_port_mac);
@@ -814,8 +823,8 @@ bool ParamTraits<base::SharedMemoryHandle>::Read(const base::Pickle* m,
   if (!ReadParam(m, iter, &handle_win))
     return false;
 #elif defined(OS_FUCHSIA)
-  HandleFuchsia handle_fuchsia;
-  if (!ReadParam(m, iter, &handle_fuchsia))
+  zx::vmo vmo;
+  if (!ReadParam(m, iter, &vmo))
     return false;
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   MachPortMac mach_port_mac;
@@ -848,8 +857,7 @@ bool ParamTraits<base::SharedMemoryHandle>::Read(const base::Pickle* m,
   *r = base::SharedMemoryHandle(handle_win.get_handle(),
                                 static_cast<size_t>(size), guid);
 #elif defined(OS_FUCHSIA)
-  *r = base::SharedMemoryHandle(handle_fuchsia.get_handle(),
-                                static_cast<size_t>(size), guid);
+  *r = base::SharedMemoryHandle(vmo.release(), static_cast<size_t>(size), guid);
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   *r = base::SharedMemoryHandle(mach_port_mac.get_mach_port(),
                                 static_cast<size_t>(size), guid);
@@ -994,9 +1002,8 @@ void ParamTraits<base::subtle::PlatformSharedMemoryRegion>::Write(
   HandleWin handle_win(h.Get());
   WriteParam(m, handle_win);
 #elif defined(OS_FUCHSIA)
-  zx::handle h = const_cast<param_type&>(p).PassPlatformHandle();
-  HandleFuchsia handle_fuchsia(h.get());
-  WriteParam(m, handle_fuchsia);
+  zx::vmo vmo = const_cast<param_type&>(p).PassPlatformHandle();
+  WriteParam(m, vmo);
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   base::mac::ScopedMachSendRight h =
       const_cast<param_type&>(p).PassPlatformHandle();
@@ -1046,11 +1053,11 @@ bool ParamTraits<base::subtle::PlatformSharedMemoryRegion>::Read(
   *r = base::subtle::PlatformSharedMemoryRegion::Take(
       base::win::ScopedHandle(handle_win.get_handle()), mode, size, guid);
 #elif defined(OS_FUCHSIA)
-  HandleFuchsia handle_fuchsia;
-  if (!ReadParam(m, iter, &handle_fuchsia))
+  zx::vmo vmo;
+  if (!ReadParam(m, iter, &vmo))
     return false;
-  *r = base::subtle::PlatformSharedMemoryRegion::Take(
-      zx::vmo(handle_fuchsia.get_handle()), mode, size, guid);
+  *r = base::subtle::PlatformSharedMemoryRegion::Take(std::move(vmo), mode,
+                                                      size, guid);
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   MachPortMac mach_port_mac;
   if (!ReadParam(m, iter, &mach_port_mac))