NaCl: enable meta-based validation for shared libraries.

ppapi/native_client/src/trusted/plugin/service_runtime.cc

Index: ppapi/native_client/src/trusted/plugin/service_runtime.cc
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index ba21a4952873dc232a9222e1836daefc9ba34f93..569b5fadf59d541981ae4d18d319f2538f9cee1c 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -46,11 +46,10 @@
 #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"
@@ -172,11 +171,11 @@ bool PluginReverseInterface::EnumerateManifestKeys(
 // 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",
@@ -228,8 +227,8 @@ bool PluginReverseInterface::OpenManifestEntry(nacl::string url_key,
   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
@@ -267,7 +266,7 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
     // 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;
   }
@@ -294,7 +293,7 @@ void PluginReverseInterface::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_);
@@ -321,7 +320,8 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
       }
       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");
@@ -347,7 +347,7 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
     } 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.");
@@ -366,16 +366,17 @@ void PluginReverseInterface::StreamAsFile_MainThreadContinuation(
 
   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");
   }
@@ -397,16 +398,16 @@ void PluginReverseInterface::BitcodeTranslate_MainThreadContinuation(
     // 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");
   }