Ensure that any IPC sent from a child process that couldn't be deserialized causes that process to be killed.

Today we do this only for a subset of IPCs and not all process types.

R=jar@chromium.org, tsepez@chromium.org

Review URL: https://codereview.chromium.org/283313002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270839 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 40 insertions, 6 deletions

diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index 5c4d743..7e32018 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -64,6 +64,10 @@ bool ChannelProxy::Context::TryFilters(const Message& message) {
 #endif
 
   if (message_filter_router_->TryFilters(message)) {
+    if (message.dispatch_error()) {
+      listener_task_runner_->PostTask(
+          FROM_HERE, base::Bind(&Context::OnDispatchBadMessage, this, message));
+    }
 #ifdef IPC_MESSAGE_LOG_ENABLED
     if (logger->Enabled())
       logger->OnPostDispatchMessage(message, channel_id_);
@@ -267,6 +271,8 @@ void ChannelProxy::Context::OnDispatchMessage(const Message& message) {
 #endif
 
   listener_->OnMessageReceived(message);
+  if (message.dispatch_error())
+    listener_->OnBadMessageReceived(message);
 
 #ifdef IPC_MESSAGE_LOG_ENABLED
   if (logger->Enabled())
@@ -290,6 +296,12 @@ void ChannelProxy::Context::OnDispatchError() {
     listener_->OnChannelError();
 }
 
+// Called on the listener's thread
+void ChannelProxy::Context::OnDispatchBadMessage(const Message& message) {
+  if (listener_)
+    listener_->OnBadMessageReceived(message);
+}
+
 //-----------------------------------------------------------------------------
 
 ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 375a42b..0a3a5d2 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -180,6 +180,7 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe {
     void AddFilter(MessageFilter* filter);
     void OnDispatchConnected();
     void OnDispatchError();
+    void OnDispatchBadMessage(const Message& message);
 
     scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner_;
     Listener* listener_;
diff --git a/ipc/ipc_channel_reader.cc b/ipc/ipc_channel_reader.cc
index 401e4e1..9a3cc3c 100644
--- a/ipc/ipc_channel_reader.cc
+++ b/ipc/ipc_channel_reader.cc
@@ -95,6 +95,8 @@ bool ChannelReader::DispatchInputData(const char* input_data,
         HandleInternalMessage(m);
       else
         listener_->OnMessageReceived(m);
+      if (m.dispatch_error())
+        listener_->OnBadMessageReceived(m);
       p = message_tail;
     } else {
       // Last message is partial.
diff --git a/ipc/ipc_listener.h b/ipc/ipc_listener.h
index 9189eec..733bc46 100644
--- a/ipc/ipc_listener.h
+++ b/ipc/ipc_listener.h
@@ -28,6 +28,9 @@ class IPC_EXPORT Listener {
   // This method is not called when a channel is closed normally.
   virtual void OnChannelError() {}
 
+  // Called when a message's deserialization failed.
+  virtual void OnBadMessageReceived(const Message& message) {}
+
 #if defined(OS_POSIX)
   // Called on the server side when a channel that listens for connections
   // denies an attempt to connect.
diff --git a/ipc/ipc_message.cc b/ipc/ipc_message.cc
index f7fe827..1ac4d6e 100644
--- a/ipc/ipc_message.cc
+++ b/ipc/ipc_message.cc
@@ -47,7 +47,7 @@ Message::Message()
   header()->num_fds = 0;
   header()->pad = 0;
 #endif
-  InitLoggingVariables();
+  Init();
 }
 
 Message::Message(int32 routing_id, uint32 type, PriorityValue priority)
@@ -60,21 +60,22 @@ Message::Message(int32 routing_id, uint32 type, PriorityValue priority)
   header()->num_fds = 0;
   header()->pad = 0;
 #endif
-  InitLoggingVariables();
+  Init();
 }
 
 Message::Message(const char* data, int data_len) : Pickle(data, data_len) {
-  InitLoggingVariables();
+  Init();
 }
 
 Message::Message(const Message& other) : Pickle(other) {
-  InitLoggingVariables();
+  Init();
 #if defined(OS_POSIX)
   file_descriptor_set_ = other.file_descriptor_set_;
 #endif
 }
 
-void Message::InitLoggingVariables() {
+void Message::Init() {
+  dispatch_error_ = false;
 #ifdef IPC_MESSAGE_LOG_ENABLED
   received_time_ = 0;
   dont_log_ = false;
diff --git a/ipc/ipc_message.h b/ipc/ipc_message.h
index ea6cda6..e4b6208 100644
--- a/ipc/ipc_message.h
+++ b/ipc/ipc_message.h
@@ -121,6 +121,14 @@ class IPC_EXPORT Message : public Pickle {
     return (header()->flags & PUMPING_MSGS_BIT) != 0;
   }
 
+  void set_dispatch_error() const {
+    dispatch_error_ = true;
+  }
+
+  bool dispatch_error() const {
+    return dispatch_error_;
+  }
+
   uint32 type() const {
     return header()->type;
   }
@@ -236,7 +244,10 @@ class IPC_EXPORT Message : public Pickle {
     return headerT<Header>();
   }
 
-  void InitLoggingVariables();
+  void Init();
+
+  // Used internally to support IPC::Listener::OnBadMessageReceived.
+  mutable bool dispatch_error_;
 
 #if defined(OS_POSIX)
   // The set of file descriptors associated with this message.
diff --git a/ipc/ipc_message_macros.h b/ipc/ipc_message_macros.h
index d5ffff7..5bc1ad4 100644
--- a/ipc/ipc_message_macros.h
+++ b/ipc/ipc_message_macros.h
@@ -933,6 +933,8 @@
         TRACK_RUN_IN_IPC_HANDLER(member_func);                                 \
         msg_is_ok__ = msg_class::Dispatch(&ipc_message__, obj, this,           \
                                           param__, &member_func);              \
+        if (!msg_is_ok__)                                                      \
+          ipc_message__.set_dispatch_error();                                  \
       }                                                                        \
       break;
 
@@ -944,6 +946,8 @@
         TRACK_RUN_IN_IPC_HANDLER(member_func);                                 \
         msg_is_ok__ = msg_class::DispatchDelayReply(&ipc_message__, obj,       \
                                                   param__, &member_func);      \
+        if (!msg_is_ok__)                                                      \
+          ipc_message__.set_dispatch_error();                                  \
       }                                                                        \
       break;