Do PPB_FileIO Query and Read in the plugin process.

content/renderer/pepper/pepper_file_io_host.cc

Index: content/renderer/pepper/pepper_file_io_host.cc
diff --git a/content/renderer/pepper/pepper_file_io_host.cc b/content/renderer/pepper/pepper_file_io_host.cc
index 934a336844098b157c3c074f68922959fadb136a..279423869cccae5f0c9ebd67cea32912bd963c12 100644
--- a/content/renderer/pepper/pepper_file_io_host.cc
+++ b/content/renderer/pepper/pepper_file_io_host.cc
@@ -58,6 +58,7 @@ PepperFileIOHost::PepperFileIOHost(RendererPpapiHost* host,
                                    PP_Instance instance,
                                    PP_Resource resource)
     : ResourceHost(host->GetPpapiHost(), instance, resource),
+      renderer_ppapi_host_(host),
       file_(base::kInvalidPlatformFileValue),
       file_system_type_(PP_FILESYSTEMTYPE_INVALID),
       quota_policy_(quota::kQuotaLimitTypeUnknown),
@@ -80,12 +81,8 @@ int32_t PepperFileIOHost::OnResourceMessageReceived(
   IPC_BEGIN_MESSAGE_MAP(PepperFileIOHost, msg)
     PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Open,
                                       OnHostMsgOpen)
-    PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_FileIO_Query,
-                                        OnHostMsgQuery)
     PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Touch,
                                       OnHostMsgTouch)
-    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Read,
-                                      OnHostMsgRead)
     PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Write,
                                       OnHostMsgWrite)
     PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_SetLength,
@@ -160,28 +157,6 @@ int32_t PepperFileIOHost::OnHostMsgOpen(
   return PP_OK_COMPLETIONPENDING;
 }
 
-int32_t PepperFileIOHost::OnHostMsgQuery(
-    ppapi::host::HostMessageContext* context) {
-  int32_t rv = state_manager_.CheckOperationState(
-      FileIOStateManager::OPERATION_EXCLUSIVE, true);
-  if (rv != PP_OK)
-    return rv;
-
-  if (!plugin_delegate_)
-    return PP_ERROR_FAILED;
-
-  if (!base::FileUtilProxy::GetFileInfoFromPlatformFile(
-          plugin_delegate_->GetFileThreadMessageLoopProxy().get(),
-          file_,
-          base::Bind(&PepperFileIOHost::ExecutePlatformQueryCallback,
-                     weak_factory_.GetWeakPtr(),
-                     context->MakeReplyMessageContext())))
-    return PP_ERROR_FAILED;
-
-  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
-  return PP_OK_COMPLETIONPENDING;
-}
-
 int32_t PepperFileIOHost::OnHostMsgTouch(
     ppapi::host::HostMessageContext* context,
     PP_Time last_access_time,
@@ -222,42 +197,6 @@ int32_t PepperFileIOHost::OnHostMsgTouch(
   return PP_OK_COMPLETIONPENDING;
 }
 
-int32_t PepperFileIOHost::OnHostMsgRead(
-    ppapi::host::HostMessageContext* context,
-    int64_t offset,
-    int32_t max_read_length) {
-  int32_t rv = state_manager_.CheckOperationState(
-      FileIOStateManager::OPERATION_READ, true);
-  if (rv != PP_OK)
-    return rv;
-
-  // Validate max_read_length before allocating below. This value is coming from
-  // the untrusted plugin.
-  if (max_read_length < 0) {
-    ReplyMessageContext reply_context = context->MakeReplyMessageContext();
-    reply_context.params.set_result(PP_ERROR_FAILED);
-    host()->SendReply(reply_context,
-                      PpapiPluginMsg_FileIO_ReadReply(std::string()));
-    return PP_OK_COMPLETIONPENDING;
-  }
-
-  if (!plugin_delegate_)
-    return PP_ERROR_FAILED;
-
-  if (!base::FileUtilProxy::Read(
-          plugin_delegate_->GetFileThreadMessageLoopProxy().get(),
-          file_,
-          offset,
-          max_read_length,
-          base::Bind(&PepperFileIOHost::ExecutePlatformReadCallback,
-                     weak_factory_.GetWeakPtr(),
-                     context->MakeReplyMessageContext())))
-    return PP_ERROR_FAILED;
-
-  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_READ);
-  return PP_OK_COMPLETIONPENDING;
-}
-
 int32_t PepperFileIOHost::OnHostMsgWrite(
     ppapi::host::HostMessageContext* context,
     int64_t offset,
@@ -414,16 +353,13 @@ int32_t PepperFileIOHost::OnHostMsgRequestOSFileHandle(
       quota_policy_ != quota::kQuotaLimitTypeUnlimited)
     return PP_ERROR_FAILED;
 
-  RendererPpapiHost* renderer_ppapi_host =
-      RendererPpapiHost::GetForPPInstance(pp_instance());
-
   // Whitelist to make it privately accessible.
   if (!GetContentClient()->renderer()->IsPluginAllowedToCallRequestOSFileHandle(
-          renderer_ppapi_host->GetContainerForInstance(pp_instance())))
+          renderer_ppapi_host_->GetContainerForInstance(pp_instance())))
     return PP_ERROR_NOACCESS;
 
   IPC::PlatformFileForTransit file =
-      renderer_ppapi_host->ShareHandleWithRemote(file_, false);
+      renderer_ppapi_host_->ShareHandleWithRemote(file_, false);
   if (file == IPC::InvalidPlatformFileForTransit())
     return PP_ERROR_FAILED;
   ppapi::host::ReplyMessageContext reply_context =
@@ -476,11 +412,22 @@ void PepperFileIOHost::ExecutePlatformOpenFileCallback(
   file_ = file.ReleaseValue();
 
   DCHECK(!quota_file_io_.get());
-  if (file_ != base::kInvalidPlatformFileValue &&
-      (file_system_type_ == PP_FILESYSTEMTYPE_LOCALTEMPORARY ||
-       file_system_type_ == PP_FILESYSTEMTYPE_LOCALPERSISTENT)) {
-    quota_file_io_.reset(new webkit::ppapi::QuotaFileIO(
-        pp_instance(), file_, file_system_url_, file_system_type_));
+  if (file_ != base::kInvalidPlatformFileValue) {
+    if (file_system_type_ == PP_FILESYSTEMTYPE_LOCALTEMPORARY ||
+        file_system_type_ == PP_FILESYSTEMTYPE_LOCALPERSISTENT) {
+      quota_file_io_.reset(new webkit::ppapi::QuotaFileIO(
+          pp_instance(), file_, file_system_url_, file_system_type_));
+    }
+
+    IPC::PlatformFileForTransit file_for_transit =
+        renderer_ppapi_host_->ShareHandleWithRemote(file_, false);

[teravest, 2013/07/12 16:07:50]

What's your plan for preventing writes to file_ in the plugin?
I don't know much about NaClDesc instances, but I think there are only two
options:
  1. Open two files (one with writes, one that's read-only), and send the
read-only file descriptor to the plugin.
  2. Do some management with NaClDesc to prevent writes to the file descriptor
on the plugin side.

Dave pointed out that we should be careful to do the right thing with trusted
plugins, so maybe we can't rely on managing file descriptor permissions with
NaClDesc?

[bbudge, 2013/07/12 17:19:15]

Good question! I hadn't dealt with this since I originally intended to implement
plugin-side write in this patch.

I think I'll have to open two file descriptors, share the read-only one, and
close it on the host side.

[bbudge, 2013/07/12 18:15:18]

I looked through the code and the NaClDesc does check the flags before writing
with the file descriptor. So although the plugin-side fd has write capability,
if we eliminate these flags on the host side, it shouldn't be possible for
untrusted code to write to it.

[teravest, 2013/07/15 16:25:12]

Cool. I don't see where that's happening now; are you going to update this patch
to do that?

+    if (!(file_for_transit == IPC::InvalidPlatformFileForTransit())) {
+      // Pass a good file descriptor over to the plugin process. This is used
+      // in the plugin for any other file operations that can be done there.
+      ppapi::proxy::SerializedHandle file_handle;
+      file_handle.set_file_handle(file_for_transit, open_flags_);
+      reply_context.params.AppendHandle(file_handle);
+    }
   }
 
   reply_context.params.set_result(pp_error);
@@ -500,36 +447,6 @@ void PepperFileIOHost::ExecutePlatformOpenFileSystemURLCallback(
   ExecutePlatformOpenFileCallback(reply_context, error_code, file);
 }
 
-void PepperFileIOHost::ExecutePlatformQueryCallback(
-    ppapi::host::ReplyMessageContext reply_context,
-    base::PlatformFileError error_code,
-    const base::PlatformFileInfo& file_info) {
-  PP_FileInfo pp_info;
-  ppapi::PlatformFileInfoToPepperFileInfo(file_info, file_system_type_,
-                                          &pp_info);
-
-  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);
-  reply_context.params.set_result(pp_error);
-  host()->SendReply(reply_context,
-                    PpapiPluginMsg_FileIO_QueryReply(pp_info));
-  state_manager_.SetOperationFinished();
-}
-
-void PepperFileIOHost::ExecutePlatformReadCallback(
-    ppapi::host::ReplyMessageContext reply_context,
-    base::PlatformFileError error_code,
-    const char* data, int bytes_read) {
-  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);
-
-  // Only send the amount of data in the string that was actually read.
-  std::string buffer;
-  if (pp_error == PP_OK)
-    buffer.append(data, bytes_read);
-  reply_context.params.set_result(ErrorOrByteNumber(pp_error, bytes_read));
-  host()->SendReply(reply_context, PpapiPluginMsg_FileIO_ReadReply(buffer));
-  state_manager_.SetOperationFinished();
-}
-
 void PepperFileIOHost::ExecutePlatformWriteCallback(
     ppapi::host::ReplyMessageContext reply_context,
     base::PlatformFileError error_code,