Bug 1289718 - Make errno handling more sane.
MozReview-Commit-ID: I1llpEBgDP6
--- 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);
}