Diagnose EBADF before EFAULT

src/trusted/service_runtime/nacl_syscall_common.c

Index: src/trusted/service_runtime/nacl_syscall_common.c
diff --git a/src/trusted/service_runtime/nacl_syscall_common.c b/src/trusted/service_runtime/nacl_syscall_common.c
index d77a51d5fdee32a5ecc0634838732df3f17158ae..21124eaab59a480054b74f3450135d7587ae9f41 100644
--- a/src/trusted/service_runtime/nacl_syscall_common.c
+++ b/src/trusted/service_runtime/nacl_syscall_common.c
@@ -725,18 +725,19 @@ int32_t NaClCommonSysGetdents(struct NaClAppThread *natp,
 
   NaClSysCommonThreadSyscallEnter(natp);
 
-  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) dirp, count);
-  if (kNaClBadAddress == sysaddr) {
-    NaClLog(4, " illegal address for directory data\n");
-    retval = -NACL_ABI_EFAULT;
-    goto cleanup;
-  }
   ndp = NaClGetDesc(natp->nap, d);
   if (NULL == ndp) {
     retval = -NACL_ABI_EBADF;
     goto cleanup;
   }
 
+  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) dirp, count);
+  if (kNaClBadAddress == sysaddr) {
+    NaClLog(4, " illegal address for directory data\n");
+    retval = -NACL_ABI_EFAULT;
+    goto cleanup_unref;
+  }
+
   /*
    * Clamp count to INT32_MAX to avoid the possibility of Getdents returning
    * a value that is outside the range of an int32.
@@ -762,6 +763,8 @@ int32_t NaClCommonSysGetdents(struct NaClAppThread *natp,
   } else {
     NaClLog(4, "getdents returned %d\n", retval);
   }
+
+cleanup_unref:
   NaClDescUnref(ndp);
 
 cleanup:
@@ -788,17 +791,19 @@ int32_t NaClCommonSysRead(struct NaClAppThread  *natp,
 
   NaClSysCommonThreadSyscallEnter(natp);
 
-  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) buf, count);
-  if (kNaClBadAddress == sysaddr) {
-    retval = -NACL_ABI_EFAULT;
-    goto cleanup;
-  }
   ndp = NaClGetDesc(natp->nap, d);
   if (NULL == ndp) {
     retval = -NACL_ABI_EBADF;
     goto cleanup;
   }
 
+  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) buf, count);
+  if (kNaClBadAddress == sysaddr) {
+    NaClDescUnref(ndp);
+    retval = -NACL_ABI_EFAULT;
+    goto cleanup;
+  }
+
   /*
    * The maximum length for read and write is INT32_MAX--anything larger and
    * the return value would overflow. Passing larger values isn't an error--
@@ -845,8 +850,15 @@ int32_t NaClCommonSysWrite(struct NaClAppThread *natp,
 
   NaClSysCommonThreadSyscallEnter(natp);
 
+  ndp = NaClGetDesc(natp->nap, d);
+  if (NULL == ndp) {
+    retval = -NACL_ABI_EBADF;
+    goto cleanup;
+  }
+
   sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) buf, count);
   if (kNaClBadAddress == sysaddr) {
+    NaClDescUnref(ndp);
     retval = -NACL_ABI_EFAULT;
     goto cleanup;
   }
@@ -854,12 +866,6 @@ int32_t NaClCommonSysWrite(struct NaClAppThread *natp,
   NaClLog(4, "In NaClSysWrite(%d, %.*s, %"NACL_PRIdS")\n",
           d, (int) count, (char *) sysaddr, count);
 
-  ndp = NaClGetDesc(natp->nap, d);
-  if (NULL == ndp) {
-    retval = -NACL_ABI_EBADF;
-    goto cleanup;
-  }
-
   /*
    * The maximum length for read and write is INT32_MAX--anything larger and
    * the return value would overflow. Passing larger values isn't an error--
@@ -904,18 +910,19 @@ int32_t NaClCommonSysLseek(struct NaClAppThread *natp,
 
   NaClSysCommonThreadSyscallEnter(natp);
 
+  ndp = NaClGetDesc(natp->nap, d);
+  if (NULL == ndp) {
+    retval = -NACL_ABI_EBADF;
+    goto cleanup;
+  }
+
   sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) offp, sizeof offset);
   if (kNaClBadAddress == sysaddr) {
     retval = -NACL_ABI_EFAULT;
-    goto cleanup;
+    goto cleanup_unref;
   }
   offset = *(nacl_abi_off_t volatile *) sysaddr;
   NaClLog(4, "offset 0x%08"NACL_PRIxNACL_OFF"\n", offset);
-  ndp = NaClGetDesc(natp->nap, d);
-  if (NULL == ndp) {
-    retval = -NACL_ABI_EBADF;
-    goto cleanup;
-  }
 
   retval64 = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
               Seek)(ndp, (nacl_off64_t) offset, whence);
@@ -925,6 +932,7 @@ int32_t NaClCommonSysLseek(struct NaClAppThread *natp,
     *(nacl_abi_off_t volatile *) sysaddr = retval64;
     retval = 0;
   }
+cleanup_unref:
   NaClDescUnref(ndp);
 cleanup:
   NaClSysCommonThreadSyscallLeave(natp);
@@ -1003,21 +1011,21 @@ int32_t NaClCommonSysFstat(struct NaClAppThread *natp,
           " sizeof(struct nacl_abi_stat) = %"NACL_PRIdS" (0x%"NACL_PRIxS")\n",
           sizeof *nasp, sizeof *nasp);
 
-  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) nasp, sizeof *nasp);
-  if (kNaClBadAddress == sysaddr) {
-    NaClLog(4, "bad addr\n");
-    retval = -NACL_ABI_EFAULT;
-    goto cleanup;
-  }
-
   ndp = NaClGetDesc(natp->nap, d);
   if (NULL == ndp) {
     NaClLog(4, "bad desc\n");
     retval = -NACL_ABI_EBADF;
     goto cleanup;
   }
-  retval = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
-            Fstat)(ndp, (struct nacl_abi_stat *) sysaddr);
+
+  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) nasp, sizeof *nasp);
+  if (kNaClBadAddress == sysaddr) {
+    NaClLog(4, "bad addr\n");
+    retval = -NACL_ABI_EFAULT;
+  } else {
+    retval = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
+              Fstat)(ndp, (struct nacl_abi_stat *) sysaddr);
+  }
 
   NaClDescUnref(ndp);
 cleanup: