nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1885218 - Add more logging for fatal errors during Linux sandbox process launch. r=gcp

Browse source 
In this code there are conditions where we can't proceed, but simply
`MOZ_CRASH`ing isn't ideal, because the crash reporter doesn't work
here, and on non-debug builds the crash string isn't printed to the
terminal, so we get bug reports where we know that something crashed
somewhere but not much else.

Because we already have the `SANDBOX_LOG` macros (which are intended to
be async signal safe), this patch uses them to add some more logging.

This patch also makes sure that `WriteStringToFile` always sets errno to
something when returning failure, because that matters for some of this
new logging.

Differential Revision: https://phabricator.services.mozilla.com/D204574
This commit is contained in:
Jed Davis 2024-03-15 23:20:32 +00:00
parent 9d05fed6bd
commit 7d1252a369
1 changed files with 23 additions and 2 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

25
security/sandbox/linux/launch/SandboxLaunch.cpp
View file

@ -511,6 +511,8 @@ static pid_t ForkWithFlags(int aFlags) {
return ret;
}
// Returns true for success, or returns false and sets errno on
// failure. Intended only for procfs pseudo-files.
static bool WriteStringToFile(const char* aPath, const char* aStr,
const size_t aLen) {
int fd = open(aPath, O_WRONLY);
@ -519,6 +521,11 @@ static bool WriteStringToFile(const char* aPath, const char* aStr,
}
ssize_t written = write(fd, aStr, aLen);
if (close(fd) != 0 || written != ssize_t(aLen)) {
// procfs shouldn't ever cause a short write, but ensure that
// errno is set to something distinctive if it does
if (written >= 0) {
errno = EMSGSIZE;
}
return false;
}
return true;
@ -537,6 +544,7 @@ static void ConfigureUserNamespace(uid_t uid, gid_t gid) {
len = static_cast<size_t>(SafeSPrintf(buf, "%d %d 1", uid, uid));
MOZ_RELEASE_ASSERT(len < sizeof(buf));
if (!WriteStringToFile("/proc/self/uid_map", buf, len)) {
SANDBOX_LOG_ERRNO("writing /proc/self/uid_map");
MOZ_CRASH("Failed to write /proc/self/uid_map");
}
@ -549,6 +557,7 @@ static void ConfigureUserNamespace(uid_t uid, gid_t gid) {
len = static_cast<size_t>(SafeSPrintf(buf, "%d %d 1", gid, gid));
MOZ_RELEASE_ASSERT(len < sizeof(buf));
if (!WriteStringToFile("/proc/self/gid_map", buf, len)) {
SANDBOX_LOG_ERRNO("writing /proc/self/gid_map");
MOZ_CRASH("Failed to write /proc/self/gid_map");
}
}
@ -641,6 +650,9 @@ void SandboxLaunch::StartChrootServer() {
char msg;
ssize_t msgLen = HANDLE_EINTR(read(mChrootServer, &msg, 1));
if (msgLen < 0) {
SANDBOX_LOG_ERRNO("chroot server couldn't read request");
}
if (msgLen == 0) {
// Process exited before chrooting (or chose not to chroot?).
_exit(0);
@ -653,7 +665,10 @@ void SandboxLaunch::StartChrootServer() {
// exits at the end of this function, and which is always
// unwriteable.
int rv = chroot("/proc/self/fdinfo");
MOZ_RELEASE_ASSERT(rv == 0);
if (rv != 0) {
SANDBOX_LOG_ERRNO("chroot");
MOZ_CRASH("chroot failed");
}
// Drop CAP_SYS_CHROOT ASAP. This must happen before responding;
// the main child won't be able to waitpid(), so it could start
@ -664,10 +679,16 @@ void SandboxLaunch::StartChrootServer() {
// remove that. (Note: if the process can obtain directory fds, for
// example via SandboxBroker, it must be blocked from using fchdir.)
rv = chdir("/");
MOZ_RELEASE_ASSERT(rv == 0);
if (rv != 0) {
SANDBOX_LOG_ERRNO("chdir(\"/\")");
MOZ_CRASH("chdir(\"/\") failed");
}
msg = kSandboxChrootResponse;
msgLen = HANDLE_EINTR(write(mChrootServer, &msg, 1));
if (msgLen < 0) {
SANDBOX_LOG_ERRNO("chroot server couldn't send response");
}
MOZ_RELEASE_ASSERT(msgLen == 1);
_exit(0);
}