Bug 1289718 - Make errno handling more sane. draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Mon, 05 Sep 2016 19:59:55 +0200
changeset 409941 322257ea40fd5d8bcb0f8999b955169230b35d0a
parent 409940 cc5e649bc384df81f2ed724162cced4878a1a6e1
child 530472 a59725bfb87d55321194df8ec2a5fe34a70903f5
push id28613
push usergpascutto@mozilla.com
push dateMon, 05 Sep 2016 18:00:21 +0000
bugs1289718
milestone51.0a1
Bug 1289718 - Make errno handling more sane. MozReview-Commit-ID: I1llpEBgDP6
security/sandbox/linux/SandboxBrokerClient.cpp
security/sandbox/linux/broker/SandboxBroker.cpp
--- a/security/sandbox/linux/SandboxBrokerClient.cpp
+++ b/security/sandbox/linux/SandboxBrokerClient.cpp
@@ -124,42 +124,42 @@ SandboxBrokerClient::DoCall(const Reques
     return -recvErrno;
   }
   if (recvd == 0) {
     SANDBOX_LOG_ERROR("Unexpected EOF, op %d flags 0%o path %s",
                       aReq->mOp, aReq->mFlags, path);
     return -EIO;
   }
   // mError is positive for failure here, we invert the logic
-  if (resp.mError > 0) {
+  if (resp.mError < 0) {
     // If the operation fails, the return payload will be empty;
     // adjust the iov_len for the following assertion.
     ios[1].iov_len = 0;
   }
   MOZ_ASSERT(static_cast<size_t>(recvd) <= ios[0].iov_len + ios[1].iov_len);
-  if (resp.mError <= 0) {
+  if (resp.mError >= 0) {
     // Success!
     if (expectFd) {
       MOZ_ASSERT(openedFd >= 0);
       return openedFd;
     }
-    return -resp.mError;
+    return resp.mError;
   }
   if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {
     // Keep in mind that "rejected" files can include ones that don't
     // actually exist, if it's something that's optional or part of a
     // search path (e.g., shared libraries).  In those cases, this
     // error message is expected.
     SANDBOX_LOG_ERROR("Rejected errno %d op %d flags 0%o path %s",
                       resp.mError, aReq->mOp, aReq->mFlags, path);
   }
   if (openedFd >= 0) {
     close(openedFd);
   }
-  return -resp.mError;
+  return resp.mError;
 }
 
 int
 SandboxBrokerClient::Open(const char* aPath, int aFlags)
 {
   Request req = { SANDBOX_FILE_OPEN, aFlags, 0 };
   int maybeFd = DoCall(&req, aPath, nullptr, nullptr, true);
   if (maybeFd >= 0) {
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -510,17 +510,17 @@ SandboxBroker::ThreadMain(void)
       SANDBOX_LOG_ERROR("no response fd from pid %d", mChildPid);
       shutdown(mFileDesc, SHUT_RD);
       break;
     }
 
     // Initialize the response with the default failure.
     memset(&resp, 0, sizeof(resp));
     memset(&respBuf, 0, sizeof(respBuf));
-    resp.mError = EACCES;
+    resp.mError = -EACCES;
     ios[0].iov_base = &resp;
     ios[0].iov_len = sizeof(resp);
     ios[1].iov_base = nullptr;
     ios[1].iov_len = 0;
     int openedFd = -1;
 
     // Clear permissions
     int perms;
@@ -553,17 +553,17 @@ SandboxBroker::ThreadMain(void)
     } else {
       // Failed to receive intelligible paths.
       perms = 0;
     }
 
     // And now perform the operation if allowed.
     if (perms & CRASH_INSTEAD) {
       // This is somewhat nonmodular, but it works.
-      resp.mError = ENOSYS;
+      resp.mError = -ENOSYS;
     } else if (permissive || perms & MAY_ACCESS) {
       // If the operation was only allowed because of permissive mode, log it.
       if (permissive && !(perms & MAY_ACCESS)) {
         AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
       }
 
       switch(req.mOp) {
       case SANDBOX_FILE_OPEN:
@@ -571,17 +571,17 @@ SandboxBroker::ThreadMain(void)
           // Permissions for O_CREAT hardwired to 0600; if that's
           // ever a problem we can change the protocol (but really we
           // should be trying to remove uses of MAY_CREATE, not add
           // new ones).
           openedFd = open(pathBuf, req.mFlags | kRequiredOpenFlags, 0600);
           if (openedFd >= 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         } else {
           AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
         }
         break;
 
       case SANDBOX_FILE_ACCESS:
         if (permissive || AllowAccess(req.mFlags, perms)) {
@@ -593,104 +593,104 @@ SandboxBroker::ThreadMain(void)
           // passes through the syscall and always ignores the flags.
           //
           // Instead, because we've already checked the requested
           // r/w/x bits against the policy, just return success if the
           // file exists and hope that's close enough.
           if (stat(pathBuf, (struct stat*)&respBuf) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         } else {
           AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
         }
         break;
 
       case SANDBOX_FILE_STAT:
         if (DoStat(pathBuf, (struct stat*)&respBuf, req.mFlags) == 0) {
           resp.mError = 0;
           ios[1].iov_base = &respBuf;
           ios[1].iov_len = sizeof(struct stat);
         } else {
-          resp.mError = errno;
+          resp.mError = -errno;
         }
         break;
 
       case SANDBOX_FILE_CHMOD:
         if (permissive || AllowOperation(W_OK, perms)) {
           if (DoChmod(pathBuf, req.mFlags) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
 
       case SANDBOX_FILE_LINK:
       case SANDBOX_FILE_SYMLINK:
         if (permissive || AllowOperation(W_OK, perms)) {
           if (DoLink(pathBuf, pathBuf2, req.mOp) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
 
       case SANDBOX_FILE_RENAME:
         if (permissive || AllowOperation(W_OK, perms)) {
           if (DoRename(pathBuf, pathBuf2) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
 
       case SANDBOX_FILE_MKDIR:
         if (permissive || AllowOperation(W_OK | X_OK, perms)) {
           if (DoMkdir(pathBuf, req.mFlags) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
 
       case SANDBOX_FILE_UNLINK:
         if (permissive || AllowOperation(W_OK, perms)) {
           if (DoUnlink(pathBuf) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
 
       case SANDBOX_FILE_RMDIR:
         if (permissive || AllowOperation(W_OK | X_OK, perms)) {
           if (DoRmdir(pathBuf) == 0) {
             resp.mError = 0;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
 
       case SANDBOX_FILE_READLINK:
         if (permissive || AllowOperation(R_OK, perms)) {
           ssize_t respSize = DoReadlink(pathBuf, &respBuf, sizeof(respBuf));
           if (respSize >= 0) {
             // Error codes are negated in the client
-            resp.mError = -respSize;
+            resp.mError = respSize;
             ios[1].iov_base = &respBuf;
             ios[1].iov_len = respSize;
           } else {
-            resp.mError = errno;
+            resp.mError = -errno;
           }
         }
         break;
       }
     } else {
       MOZ_ASSERT(perms == 0);
       AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
     }