Diagnose EBADF before EFAULT

src/trusted/service_runtime/nacl_syscall_common.c

 /*
  * Copyright (c) 2011 The Native Client Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 /*
  * NaCl service run-time, non-platform specific system call helper routines.
  */
 #include <sys/types.h>
@@ skipping 707 matching lines @@
   struct NaClDesc *ndp;
 
   NaClLog(3,
           ("Entered NaClCommonSysGetdents(0x%08"NACL_PRIxPTR", "
            "%d, 0x%08"NACL_PRIxPTR", "
            "%"NACL_PRIdS"[0x%"NACL_PRIxS"])\n"),
           (uintptr_t) natp, d, (uintptr_t) dirp, count, count);
 
   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.
    */
   if (count > INT32_MAX) {
     count = INT32_MAX;
   }
   getdents_ret = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
                   Getdents)(ndp,
                             (void *) sysaddr,
                             count);
   if ((getdents_ret < INT32_MIN && !NaClSSizeIsNegErrno(&getdents_ret))
       || INT32_MAX < getdents_ret) {
     /* This should never happen, because we already clamped the input count */
     NaClLog(LOG_FATAL, "Overflow in Getdents: return value is %"NACL_PRIxS,
             getdents_ret);
   } else {
     retval = (int32_t) getdents_ret;
   }
   if (retval > 0) {
     NaClLog(4, "getdents returned %d bytes\n", retval);
     NaClLog(8, "getdents result: %.*s\n", retval, (char *) sysaddr);
   } else {
     NaClLog(4, "getdents returned %d\n", retval);
   }
+
+cleanup_unref:
   NaClDescUnref(ndp);
 
 cleanup:
   NaClSysCommonThreadSyscallLeave(natp);
 
   return retval;
 }
 
 int32_t NaClCommonSysRead(struct NaClAppThread  *natp,
                           int                   d,
                           void                  *buf,
                           size_t                count) {
   int32_t         retval = -NACL_ABI_EINVAL;
   ssize_t         read_result = -NACL_ABI_EINVAL;
   uintptr_t       sysaddr;
   struct NaClDesc *ndp;
 
 
   NaClLog(3,
           ("Entered NaClCommonSysRead(0x%08"NACL_PRIxPTR", "
            "%d, 0x%08"NACL_PRIxPTR", "
            "%"NACL_PRIdS"[0x%"NACL_PRIxS"])\n"),
           (uintptr_t) natp, d, (uintptr_t) buf, count, count);
 
   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--
    * we'll just clamp the request size if it's too large.
    */
   if (count > INT32_MAX) {
     count = INT32_MAX;
   }
 
   read_result = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
@@ skipping 26 matching lines @@
   struct NaClDesc *ndp;
 
   NaClLog(3,
           "Entered NaClCommonSysWrite(0x%08"NACL_PRIxPTR", "
           "%d, 0x%08"NACL_PRIxPTR", "
           "%"NACL_PRIdS"[0x%"NACL_PRIxS"])\n",
           (uintptr_t) natp, d, (uintptr_t) buf, count, count);
 
   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;
   }
 
   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--
    * we'll just clamp the request size if it's too large.
    */
   if (count > INT32_MAX) {
     count = INT32_MAX;
   }
 
   write_result = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
@@ skipping 24 matching lines @@
   int32_t         retval = -NACL_ABI_EINVAL;
   struct NaClDesc *ndp;
 
   NaClLog(3,
           ("Entered NaClCommonSysLseek(0x%08"NACL_PRIxPTR", %d,"
            " 0x%08"NACL_PRIxPTR", %d)\n"),
           (uintptr_t) natp, d, (uintptr_t) offp, whence);
 
   NaClSysCommonThreadSyscallEnter(natp);
 
-  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) offp, sizeof offset);
-  if (kNaClBadAddress == sysaddr) {
-    retval = -NACL_ABI_EFAULT;
-    goto cleanup;
-  }
-  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;
   }
 
+  sysaddr = NaClUserToSysAddrRange(natp->nap, (uintptr_t) offp, sizeof offset);
+  if (kNaClBadAddress == sysaddr) {
+    retval = -NACL_ABI_EFAULT;
+    goto cleanup_unref;
+  }
+  offset = *(nacl_abi_off_t volatile *) sysaddr;
+  NaClLog(4, "offset 0x%08"NACL_PRIxNACL_OFF"\n", offset);
+
   retval64 = (*((struct NaClDescVtbl const *) ndp->base.vtbl)->
               Seek)(ndp, (nacl_off64_t) offset, whence);
   if (NaClOff64IsNegErrno(&retval64)) {
     retval = (int32_t) retval64;
   } else {
     *(nacl_abi_off_t volatile *) sysaddr = retval64;
     retval = 0;
   }
+cleanup_unref:
   NaClDescUnref(ndp);
 cleanup:
   NaClSysCommonThreadSyscallLeave(natp);
   return retval;
 }
 
 int32_t NaClCommonSysIoctl(struct NaClAppThread *natp,
                            int                  d,
                            int                  request,
                            void                 *arg) {
@@ skipping 58 matching lines @@
            ", %d, 0x%08"NACL_PRIxPTR")\n"),
           (uintptr_t) natp,
           d, (uintptr_t) nasp);
 
   NaClSysCommonThreadSyscallEnter(natp);
 
   NaClLog(4,
           " 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:
   NaClSysCommonThreadSyscallLeave(natp);
 
   return retval;
 }
 
 int32_t NaClCommonSysStat(struct NaClAppThread  *natp,
                           const char            *pathname,
@@ skipping 1717 matching lines @@
     retval = -NACL_ABI_EBADF;
     goto cleanup;
   }
 
   retval = (*((struct NaClDescVtbl const *) desc->base.vtbl)->GetValue)(desc);
   NaClDescUnref(desc);
 cleanup:
   NaClSysCommonThreadSyscallLeave(natp);
   return retval;
 }