Skip to content

Commit

Permalink
Kernel: Do a bit more of do_exec() before disabling interrupts.
Browse files Browse the repository at this point in the history
We definitely need to replace m_executable before clearing interrupts, since
otherwise we might call ~Custody() which would make it assert in locking.
Also avoid calling FileDescriptor::metadata() repeatedly and just cache the
result from the first call.

I also added a comment at the point where we've decided to commit to the new
executable and follow through with the swap.
  • Loading branch information
awesomekling committed May 31, 2019
1 parent e6a8133 commit e33dadb
Showing 1 changed file with 11 additions and 11 deletions.
22 changes: 11 additions & 11 deletions Kernel/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,13 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
if (result.is_error())
return result.error();
auto descriptor = result.value();
auto metadata = descriptor->metadata();

if (!descriptor->metadata().may_execute(m_euid, m_gids))
if (!metadata.may_execute(m_euid, m_gids))
return -EACCES;

if (!descriptor->metadata().size) {
if (!metadata.size)
return -ENOTIMPL;
}

dword entry_eip = 0;
// FIXME: Is there a race here?
Expand All @@ -339,7 +339,7 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
#else
vmo->set_name("ELF image");
#endif
RetainPtr<Region> region = allocate_region_with_vmo(LinearAddress(), descriptor->metadata().size, vmo.copy_ref(), 0, "executable", PROT_READ);
RetainPtr<Region> region = allocate_region_with_vmo(LinearAddress(), metadata.size, vmo.copy_ref(), 0, "executable", PROT_READ);
ASSERT(region);

if (this != &current->process()) {
Expand Down Expand Up @@ -388,10 +388,17 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
return -ENOEXEC;
}

// NOTE: At this point, we've committed to the new executable.
entry_eip = loader->entry().get();
}

m_elf_loader = move(loader);
m_executable = descriptor->custody();

if (metadata.is_setuid())
m_euid = metadata.uid;
if (metadata.is_setgid())
m_egid = metadata.gid;

current->m_kernel_stack_for_signal_handler_region = nullptr;
current->m_signal_stack_user_region = nullptr;
Expand Down Expand Up @@ -435,13 +442,6 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
main_thread().m_tss.esp0 = old_esp0;
main_thread().m_tss.ss2 = m_pid;

m_executable = descriptor->custody();

if (descriptor->metadata().is_setuid())
m_euid = descriptor->metadata().uid;
if (descriptor->metadata().is_setgid())
m_egid = descriptor->metadata().gid;

#ifdef TASK_DEBUG
kprintf("Process %u (%s) exec'd %s @ %p\n", pid(), name().characters(), path.characters(), main_thread().tss().eip);
#endif
Expand Down

0 comments on commit e33dadb

Please sign in to comment.