Skip to content

Commit

Permalink
Kernel: Hoist allocation of main program FD in sys$execve()
Browse files Browse the repository at this point in the history
When executing a dynamically linked program, we need to pass the main
program executable via a file descriptor to the dynamic loader.

Before this patch, we were allocating an FD for this purpose long after
it was safe to do anything fallible. If we were unable to allocate an
FD we would simply panic the kernel(!)

We now hoist the allocation so it can fail before we've committed to
a new executable.
  • Loading branch information
awesomekling committed Sep 6, 2021
1 parent b141bfe commit f4624e4
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions Kernel/Syscalls/execve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct LoadResult {
WeakPtr<Memory::Region> stack_region;
};

static Vector<ELF::AuxiliaryValue> generate_auxiliary_vector(FlatPtr load_base, FlatPtr entry_eip, UserID uid, UserID euid, GroupID gid, GroupID egid, String executable_path, int main_program_fd);
static Vector<ELF::AuxiliaryValue> generate_auxiliary_vector(FlatPtr load_base, FlatPtr entry_eip, UserID uid, UserID euid, GroupID gid, GroupID egid, String executable_path, Optional<Process::ScopedDescriptionAllocation> const& main_program_fd_allocation);

static bool validate_stack_size(const Vector<String>& arguments, const Vector<String>& environment)
{
Expand Down Expand Up @@ -461,6 +461,11 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
auto signal_trampoline_region = TRY(load_result.space->allocate_region_with_vmobject(signal_trampoline_range, g_signal_trampoline_region->vmobject(), 0, "Signal trampoline", PROT_READ | PROT_EXEC, true));
signal_trampoline_region->set_syscall_region(true);

// (For dynamically linked executable) Allocate an FD for passing the main executable to the dynamic loader.
Optional<ScopedDescriptionAllocation> main_program_fd_allocation;
if (interpreter_description)
main_program_fd_allocation = TRY(m_fds.allocate());

// We commit to the new executable at this point. There is no turning back!

// Prevent other processes from attaching to us with ptrace while we're doing this.
Expand Down Expand Up @@ -521,15 +526,11 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
file_description_metadata = {};
});

int main_program_fd = -1;
if (interpreter_description) {
auto main_program_fd_wrapper = m_fds.allocate().release_value();
VERIFY(main_program_fd_wrapper.fd >= 0);
if (main_program_fd_allocation.has_value()) {
auto seek_result = main_program_description->seek(0, SEEK_SET);
VERIFY(!seek_result.is_error());
main_program_description->set_readable(true);
m_fds[main_program_fd_wrapper.fd].set(move(main_program_description), FD_CLOEXEC);
main_program_fd = main_program_fd_wrapper.fd;
m_fds[main_program_fd_allocation->fd].set(move(main_program_description), FD_CLOEXEC);
}

new_main_thread = nullptr;
Expand All @@ -543,7 +544,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
}
VERIFY(new_main_thread);

auto auxv = generate_auxiliary_vector(load_result.load_base, load_result.entry_eip, uid(), euid(), gid(), egid(), path, main_program_fd);
auto auxv = generate_auxiliary_vector(load_result.load_base, load_result.entry_eip, uid(), euid(), gid(), egid(), path, main_program_fd_allocation);

// NOTE: We create the new stack before disabling interrupts since it will zero-fault
// and we don't want to deal with faults after this point.
Expand Down Expand Up @@ -627,7 +628,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
return KSuccess;
}

static Vector<ELF::AuxiliaryValue> generate_auxiliary_vector(FlatPtr load_base, FlatPtr entry_eip, UserID uid, UserID euid, GroupID gid, GroupID egid, String executable_path, int main_program_fd)
static Vector<ELF::AuxiliaryValue> generate_auxiliary_vector(FlatPtr load_base, FlatPtr entry_eip, UserID uid, UserID euid, GroupID gid, GroupID egid, String executable_path, Optional<Process::ScopedDescriptionAllocation> const& main_program_fd_allocation)
{
Vector<ELF::AuxiliaryValue> auxv;
// PHDR/EXECFD
Expand Down Expand Up @@ -658,7 +659,8 @@ static Vector<ELF::AuxiliaryValue> generate_auxiliary_vector(FlatPtr load_base,

auxv.append({ ELF::AuxiliaryValue::ExecFilename, executable_path });

auxv.append({ ELF::AuxiliaryValue::ExecFileDescriptor, main_program_fd });
if (main_program_fd_allocation.has_value())
auxv.append({ ELF::AuxiliaryValue::ExecFileDescriptor, main_program_fd_allocation->fd });

auxv.append({ ELF::AuxiliaryValue::Null, 0L });
return auxv;
Expand Down

0 comments on commit f4624e4

Please sign in to comment.