NaCl: enable meta-based validation for shared libraries.

ppapi/native_client/src/trusted/plugin/service_runtime.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.
  */
 
 #define NACL_LOG_MODULE_NAME "Plugin::ServiceRuntime"
 
 #include "native_client/src/trusted/plugin/service_runtime.h"
 
@@ skipping 28 matching lines @@
 // PostMessage method, so this undef must appear before any of those.
 #ifdef PostMessage
 #undef PostMessage
 #endif
 #include "native_client/src/trusted/plugin/plugin.h"
 #include "native_client/src/trusted/plugin/plugin_error.h"
 #include "native_client/src/trusted/plugin/pnacl_coordinator.h"
 #include "native_client/src/trusted/plugin/pnacl_resources.h"
 #include "native_client/src/trusted/plugin/sel_ldr_launcher_chrome.h"
 #include "native_client/src/trusted/plugin/srpc_client.h"
-
-#include "native_client/src/trusted/weak_ref/call_on_main_thread.h"
-
+#include "native_client/src/trusted/reverse_service/nacl_file_info.h"
 #include "native_client/src/trusted/service_runtime/nacl_error_code.h"
 #include "native_client/src/trusted/service_runtime/include/sys/nacl_imc_api.h"
+#include "native_client/src/trusted/weak_ref/call_on_main_thread.h"
 
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/trusted/ppb_file_io_trusted.h"
 #include "ppapi/cpp/core.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/file_io.h"
 
 namespace {
 
 // For doing crude quota enforcement on writes to temp files.
@@ skipping 101 matching lines @@
     return false;
   }
 
   return true;
 }
 
 // TODO(bsy): OpenManifestEntry should use the manifest to ResolveKey
 // and invoke StreamAsFile with a completion callback that invokes
 // GetPOSIXFileDesc.
 bool PluginReverseInterface::OpenManifestEntry(nacl::string url_key,
-                                               int32_t* out_desc) {
+                                               struct NaClFileInfo *info) {
   ErrorInfo error_info;
   bool op_complete = false;  // NB: mu_ and cv_ also controls access to this!
   OpenManifestEntryResource* to_open =
-      new OpenManifestEntryResource(url_key, out_desc,
+      new OpenManifestEntryResource(url_key, info,
                                     &error_info, &op_complete);

[dmichael (off chromium), 2013/05/15 18:37:53]

I know this isn't your fault, but this seems to warrant a comment to me: to_open
is made on the heap, but has pointers to stack variables (and |info| which I
guess comes from SRPC?). It appears that to_open will be deleted in the
continuation, and that must happen in another thread before this function exits.
So I think it's all OK, but it looks fishy at first glance.

[dmichael (off chromium), 2013/05/16 17:08:35]

Could you comment this a little, pretty please? The way this method works by
jumping threads and shuffling pointers around is unusual and a bit tricky. It
would be helpful to have a high-level comment about what it's doing (which will
also hopefully explain to the reader why it's okay to pass stack pointers to
this heap object).

[Nick Bray (chromium), 2013/05/16 17:44:15]

Done.

   CHECK(to_open != NULL);
   NaClLog(4, "PluginReverseInterface::OpenManifestEntry: %s\n",
           url_key.c_str());
   // This assumes we are not on the main thread.  If false, we deadlock.
   plugin::WeakRefCallOnMainThread(
       anchor_,
       0,
       this,
       &plugin::PluginReverseInterface::OpenManifestEntry_MainThreadContinuation,
       to_open);
@@ skipping 30 matching lines @@
   // The caller is responsible for not closing *out_desc.  If it is
   // closed prematurely, then another open could re-use the OS
   // descriptor, confusing the opened_ map.  If the caller is going to
   // want to make a NaClDesc object and transfer it etc., then the
   // caller should DUP the descriptor (but remember the original
   // value) for use by the NaClDesc object, which closes when the
   // object is destroyed.
   NaClLog(4,
           "PluginReverseInterface::OpenManifestEntry:"
           " *out_desc = %d\n",
-          *out_desc);
-  if (*out_desc == -1) {
+          info->desc);
+  if (info->desc == -1) {
     // TODO(bsy,ncbray): what else should we do with the error?  This
     // is a runtime error that may simply be a programming error in
     // the untrusted code, or it may be something else wrong w/ the
     // manifest.
     NaClLog(4,
             "OpenManifestEntry: failed for key %s, code %d (%s)\n",
             url_key.c_str(),
             error_info.error_code(),
             error_info.message().c_str());
   }
@@ skipping 17 matching lines @@
 
   std::string mapped_url;
   PnaclOptions pnacl_options;
   if (!manifest_->ResolveKey(p->url, &mapped_url,
                              &pnacl_options, p->error_info)) {
     NaClLog(4, "OpenManifestEntry_MainThreadContinuation: ResolveKey failed\n");
     // Failed, and error_info has the details on what happened.  Wake
     // up requesting thread -- we are done.
     nacl::MutexLocker take(&mu_);
     *p->op_complete_ptr = true;  // done...
-    *p->out_desc = -1;       // but failed.
+    p->file_info->desc = -1;  // but failed.
     NaClXCondVarBroadcast(&cv_);
     return;
   }
   NaClLog(4,
           "OpenManifestEntry_MainThreadContinuation: "
           "ResolveKey: %s -> %s (pnacl_translate(%d))\n",
           p->url.c_str(), mapped_url.c_str(), pnacl_options.translate());
 
   open_cont = new OpenManifestEntryResource(*p);  // copy ctor!
   CHECK(open_cont != NULL);
   open_cont->url = mapped_url;
   if (!pnacl_options.translate()) {
     pp::CompletionCallback stream_cc = WeakRefNewCallback(
         anchor_,
         this,
         &PluginReverseInterface::StreamAsFile_MainThreadContinuation,
         open_cont);
     // Normal files.
     if (!PnaclUrls::IsPnaclComponent(mapped_url)) {
       if (!plugin_->StreamAsFile(mapped_url,
                                  stream_cc.pp_completion_callback())) {
         NaClLog(4,
                 "OpenManifestEntry_MainThreadContinuation: "
                 "StreamAsFile failed\n");
         nacl::MutexLocker take(&mu_);
         *p->op_complete_ptr = true;  // done...
-        *p->out_desc = -1;       // but failed.
+        p->file_info->desc = -1;       // but failed.
         p->error_info->SetReport(ERROR_MANIFEST_OPEN,
                                  "ServiceRuntime: StreamAsFile failed");
         NaClXCondVarBroadcast(&cv_);
         return;
       }
       NaClLog(4,
               "OpenManifestEntry_MainThreadContinuation: StreamAsFile okay\n");
     } else {
       // Special PNaCl support files, that are installed on the
       // user machine.
       int32_t fd = PnaclResources::GetPnaclFD(
           plugin_,
           PnaclUrls::PnaclComponentURLToFilename(mapped_url).c_str());
       if (fd < 0) {
         // We should check earlier if the pnacl component wasn't installed
         // yet.  At this point, we can't do much anymore, so just continue
         // with an invalid fd.
         NaClLog(4,
                 "OpenManifestEntry_MainThreadContinuation: "
                 "GetReadonlyPnaclFd failed\n");
         // TODO(jvoung): Separate the error codes?
         p->error_info->SetReport(ERROR_MANIFEST_OPEN,
                                  "ServiceRuntime: GetPnaclFd failed");
       }
       nacl::MutexLocker take(&mu_);
       *p->op_complete_ptr = true;  // done!
-      *p->out_desc = fd;
+      // TODO(ncbray): more info for faster validation?
+      p->file_info->desc = fd;
       NaClXCondVarBroadcast(&cv_);
       NaClLog(4,
               "OpenManifestEntry_MainThreadContinuation: GetPnaclFd okay\n");
     }
   } else {
     // Requires PNaCl translation.
     NaClLog(4,
             "OpenManifestEntry_MainThreadContinuation: "
             "pulling down and translating.\n");
     if (plugin_->nacl_interface()->IsPnaclEnabled()) {
       pp::CompletionCallback translate_callback =
           WeakRefNewCallback(
               anchor_,
               this,
               &PluginReverseInterface::BitcodeTranslate_MainThreadContinuation,
               open_cont);
       // Will always call the callback on success or failure.
       pnacl_coordinator_.reset(
           PnaclCoordinator::BitcodeToNative(plugin_,
                                             mapped_url,
                                             pnacl_options,
                                             translate_callback));
     } else {
       nacl::MutexLocker take(&mu_);
       *p->op_complete_ptr = true;  // done...
-      *p->out_desc = -1;       // but failed.
+      p->file_info->desc = -1;  // but failed.
       p->error_info->SetReport(ERROR_PNACL_NOT_ENABLED,
                                "ServiceRuntime: GetPnaclFd failed -- pnacl not "
                                "enabled with --enable-pnacl.");
       NaClXCondVarBroadcast(&cv_);
       return;
     }
   }
   // p is deleted automatically
 }
 
 void PluginReverseInterface::StreamAsFile_MainThreadContinuation(
     OpenManifestEntryResource* p,
     int32_t result) {
   NaClLog(4,
           "Entered StreamAsFile_MainThreadContinuation\n");
 
   nacl::MutexLocker take(&mu_);
   if (result == PP_OK) {
-    NaClLog(4, "StreamAsFile_MainThreadContinuation: GetPOSIXFileDesc(%s)\n",
+    NaClLog(4, "StreamAsFile_MainThreadContinuation: GetFileInfo(%s)\n",
             p->url.c_str());
-    *p->out_desc = plugin_->GetPOSIXFileDesc(p->url);
+    *p->file_info = plugin_->GetFileInfo(p->url);
+
     NaClLog(4,
             "StreamAsFile_MainThreadContinuation: PP_OK, desc %d\n",
-            *p->out_desc);
+            p->file_info->desc);
   } else {
     NaClLog(4,
             "StreamAsFile_MainThreadContinuation: !PP_OK, setting desc -1\n");
-    *p->out_desc = -1;
+    p->file_info->desc = -1;
     p->error_info->SetReport(ERROR_MANIFEST_OPEN,
                              "Plugin StreamAsFile failed at callback");
   }
   *p->op_complete_ptr = true;
   NaClXCondVarBroadcast(&cv_);
 }
 
 
 void PluginReverseInterface::BitcodeTranslate_MainThreadContinuation(
     OpenManifestEntryResource* p,
     int32_t result) {
   NaClLog(4,
           "Entered BitcodeTranslate_MainThreadContinuation\n");
 
   nacl::MutexLocker take(&mu_);
   if (result == PP_OK) {
     // TODO(jvoung): clean this up. We are assuming that the NaClDesc is
     // a host IO desc and doing a downcast. Once the ReverseInterface
     // accepts NaClDescs we can avoid this downcast.
     NaClDesc* desc = pnacl_coordinator_->ReleaseTranslatedFD()->desc();
     struct NaClDescIoDesc* ndiodp = (struct NaClDescIoDesc*)desc;
-    *p->out_desc = ndiodp->hd->d;
+    p->file_info->desc = ndiodp->hd->d;
     pnacl_coordinator_.reset(NULL);
     NaClLog(4,
             "BitcodeTranslate_MainThreadContinuation: PP_OK, desc %d\n",
-            *p->out_desc);
+            p->file_info->desc);
   } else {
     NaClLog(4,
             "BitcodeTranslate_MainThreadContinuation: !PP_OK, "
             "setting desc -1\n");
-    *p->out_desc = -1;
+    p->file_info->desc = -1;
     // Error should have been reported by pnacl coordinator.
     NaClLog(LOG_ERROR, "PluginReverseInterface::BitcodeTranslate error.\n");
   }
   *p->op_complete_ptr = true;
   NaClXCondVarBroadcast(&cv_);
 }
 
 
 bool PluginReverseInterface::CloseManifestEntry(int32_t desc) {
   bool op_complete = false;
@@ skipping 426 matching lines @@
 
 nacl::string ServiceRuntime::GetCrashLogOutput() {
   if (NULL != subprocess_.get()) {
     return subprocess_->GetCrashLogOutput();
   } else {
     return std::string();
   }
 }
 
 }  // namespace plugin