Skip to content

Commit

Permalink
Kernel: Prevent executing I/O instructions in userspace
Browse files Browse the repository at this point in the history
All threads were running with iomapbase=0 in their TSS, which the CPU
interprets as "there's an I/O permission bitmap starting at offset 0
into my TSS".

Because of that, any bits that were 1 inside the TSS would allow the
thread to execute I/O instructions on the port with that bit index.

Fix this by always setting the iomapbase to sizeof(TSS32), and also
setting the TSS descriptor's limit to sizeof(TSS32), effectively making
the I/O permissions bitmap zero-length.

This should make it no longer possible to do I/O from userspace. :^)
  • Loading branch information
awesomekling committed Jan 1, 2020
1 parent 37329c2 commit f598bbb
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 6 deletions.
1 change: 1 addition & 0 deletions Base/usr/share/man/man1/crash.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ kinds of crashes.
* `-y`: Write to recently freed memory. (Tests an opportunistic malloc guard.)
* `-X`: Attempt to execute non-executable memory. (Not mapped with PROT\_EXEC.)
* `-U`: Attempt to trigger an x86 User Mode Instruction Prevention fault.
* `-I`: Use an x86 I/O instruction in userspace.

## Examples

Expand Down
2 changes: 2 additions & 0 deletions Kernel/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,8 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
new_main_thread->make_thread_specific_region({});

memset(&tss, 0, sizeof(TSS32));
tss.iomapbase = sizeof(TSS32);

tss.eflags = 0x0202;
tss.eip = entry_eip;
tss.cs = 0x1b;
Expand Down
8 changes: 4 additions & 4 deletions Kernel/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,10 @@ bool Scheduler::context_switch(Thread& thread)
thread.set_selector(gdt_alloc_entry());
auto& descriptor = get_gdt_entry(thread.selector());
descriptor.set_base(&thread.tss());
descriptor.set_limit(0xffff);
descriptor.set_limit(sizeof(TSS32));
descriptor.dpl = 0;
descriptor.segment_present = 1;
descriptor.granularity = 1;
descriptor.granularity = 0;
descriptor.zero = 0;
descriptor.operation_size = 1;
descriptor.descriptor_type = 0;
Expand All @@ -501,10 +501,10 @@ static void initialize_redirection()
{
auto& descriptor = get_gdt_entry(s_redirection.selector);
descriptor.set_base(&s_redirection.tss);
descriptor.set_limit(0xffff);
descriptor.set_limit(sizeof(TSS32));
descriptor.dpl = 0;
descriptor.segment_present = 1;
descriptor.granularity = 1;
descriptor.granularity = 0;
descriptor.zero = 0;
descriptor.operation_size = 1;
descriptor.descriptor_type = 0;
Expand Down
1 change: 1 addition & 0 deletions Kernel/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Thread::Thread(Process& process)
m_fpu_state = (FPUState*)kmalloc_aligned(sizeof(FPUState), 16);
memcpy(m_fpu_state, &s_clean_fpu_state, sizeof(FPUState));
memset(&m_tss, 0, sizeof(m_tss));
m_tss.iomapbase = sizeof(TSS32);

// Only IF is set when a process boots.
m_tss.eflags = 0x0202;
Expand Down
15 changes: 13 additions & 2 deletions Userland/crash.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <AK/Function.h>
#include <AK/String.h>
#include <Kernel/IO.h>
#include <Kernel/Syscall.h>
#include <stdio.h>
#include <stdlib.h>
Expand All @@ -10,7 +11,7 @@

static void print_usage_and_exit()
{
printf("usage: crash -[AsdiamfMFTtSxyXU]\n");
printf("usage: crash -[AsdiamfMFTtSxyXUI]\n");
exit(0);
}

Expand Down Expand Up @@ -99,6 +100,7 @@ int main(int argc, char** argv)
ReadFromFreedMemoryStillCachedByMalloc,
ExecuteNonExecutableMemory,
TriggerUserModeInstructionPrevention,
UseIOInstruction,
};
Mode mode = SegmentationViolation;

Expand Down Expand Up @@ -139,6 +141,8 @@ int main(int argc, char** argv)
mode = ExecuteNonExecutableMemory;
else if (String(argv[1]) == "-U")
mode = TriggerUserModeInstructionPrevention;
else if (String(argv[1]) == "-I")
mode = UseIOInstruction;
else
print_usage_and_exit();

Expand Down Expand Up @@ -330,6 +334,13 @@ int main(int argc, char** argv)
}).run(run_type);
}

if (mode == UseIOInstruction || mode == TestAllCrashTypes) {
Crash("Attempt to use an I/O instruction", [] {
u8 keyboard_status = IO::in8(0x64);
printf("Keyboard status: %#02x\n", keyboard_status);
return Crash::Failure::DidNotCrash;
}).run(run_type);
}

return 0;
}

0 comments on commit f598bbb

Please sign in to comment.