Skip to content

Commit

Permalink
Kernel+SystemServer+CrashDaemon: Better control where we put core dumps
Browse files Browse the repository at this point in the history
SystemServer now creates the /tmp/coredump and /tmp/profiler_coredumps
directories at startup, ensuring that they are owned by root, and with
basic 0755 permissions.

The kernel will also now refuse to put core dumps in a directory that
doesn't fulfill the following criteria:

- Owned by 0:0
- Directory with sticky bit not set
- 0755 permissions

Fixes SerenityOS#4435
Fixes SerenityOS#4850
  • Loading branch information
awesomekling committed Jan 10, 2021
1 parent f152b6f commit 190e0e1
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 11 deletions.
20 changes: 12 additions & 8 deletions Kernel/CoreDump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,27 @@ RefPtr<FileDescription> CoreDump::create_target_file(const Process& process, con
{
LexicalPath lexical_path(output_path);
auto output_directory = lexical_path.dirname();
if (VFS::the().open_directory(output_directory, VFS::the().root_custody()).is_error()) {
auto res = VFS::the().mkdir(output_directory, 0777, VFS::the().root_custody());
if (res.is_error())
return nullptr;
auto dump_directory = VFS::the().open_directory(output_directory, VFS::the().root_custody());
if (dump_directory.is_error()) {
dbgln("Can't find directory '{}' for core dump", output_directory);
return nullptr;
}
auto tmp_dir = VFS::the().open_directory(output_directory, VFS::the().root_custody());
if (tmp_dir.is_error())
auto dump_directory_metadata = dump_directory.value()->inode().metadata();
if (dump_directory_metadata.uid != 0 || dump_directory_metadata.gid != 0 || dump_directory_metadata.mode != 040755) {
dbgln("Refusing to put core dump in sketchy directory '{}'", output_directory);
return nullptr;
}
auto fd_or_error = VFS::the().open(
lexical_path.basename(),
O_CREAT | O_WRONLY | O_EXCL,
0, // We will enable reading from userspace when we finish generating the coredump file
*tmp_dir.value(),
*dump_directory.value(),
UidAndGid { process.uid(), process.gid() });

if (fd_or_error.is_error())
if (fd_or_error.is_error()) {
dbgln("Failed to open core dump '{}' for writing", output_path);
return nullptr;
}

return fd_or_error.value();
}
Expand Down
4 changes: 1 addition & 3 deletions Services/CrashDaemon/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ static void launch_crash_reporter(const String& coredump_path)

int main()
{
static constexpr const char* coredumps_dir = "/tmp/coredump";
mkdir(coredumps_dir, 0777);
Core::DirectoryWatcher watcher { coredumps_dir };
Core::DirectoryWatcher watcher { "/tmp/coredump" };
while (true) {
auto event = watcher.wait_for_event();
ASSERT(event.has_value());
Expand Down
26 changes: 26 additions & 0 deletions Services/SystemServer/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,30 @@ static void create_tmp_rpc_directory()
umask(old_umask);
}

static void create_tmp_coredump_directory()
{
dbgln("Creating /tmp/coredump directory");
auto old_umask = umask(0);
auto rc = mkdir("/tmp/coredump", 0755);
if (rc < 0) {
perror("mkdir(/tmp/coredump)");
ASSERT_NOT_REACHED();
}
umask(old_umask);
}

static void create_tmp_profiler_coredumps_directory()
{
dbgln("Creating /tmp/profiler_coredumps directory");
auto old_umask = umask(0);
auto rc = mkdir("/tmp/profiler_coredumps", 0755);
if (rc < 0) {
perror("mkdir(/tmp/profiler_coredumps)");
ASSERT_NOT_REACHED();
}
umask(old_umask);
}

int main(int, char**)
{
prepare_devfs();
Expand All @@ -203,6 +227,8 @@ int main(int, char**)

mount_all_filesystems();
create_tmp_rpc_directory();
create_tmp_coredump_directory();
create_tmp_profiler_coredumps_directory();
parse_boot_mode();

Core::EventLoop event_loop;
Expand Down

0 comments on commit 190e0e1

Please sign in to comment.