From 31d62f9272c511d26b6788b9b4b27bae8ff44023 Mon Sep 17 00:00:00 2001 From: Jeremy Soller Date: Tue, 6 Dec 2016 14:47:05 -0700 Subject: [PATCH] Use isohybrid to generate a USB stick friendly ISO Add assertions to verify that grants are unmapped Fix grant unmapping in exec and exit, thus fixing some crashes without network cards --- .travis.yml | 2 +- Makefile | 1 + kernel/context/memory.rs | 27 ++++++++++++++++++++++----- kernel/scheme/user.rs | 8 ++++++++ kernel/syscall/process.rs | 31 +++++++++++++++++++++++++++++-- 5 files changed, 61 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 03dbaa3..87cfbf2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,7 @@ os: - linux dist: trusty before_install: -- if [ `uname` = "Linux" ]; then sudo apt-get install -qq nasm pkg-config fuse libfuse-dev; +- if [ `uname` = "Linux" ]; then sudo apt-get install -qq nasm pkg-config fuse libfuse-dev syslinux-utils; sudo modprobe fuse; sudo chmod 666 /dev/fuse; sudo chown root:$USER /etc/fuse.conf; fi - if [ `uname` = "Darwin" ]; then brew update; brew install nasm gcc49 pkg-config diff --git a/Makefile b/Makefile index 97fc7d9..feb3edf 100644 --- a/Makefile +++ b/Makefile @@ -233,6 +233,7 @@ build/livedisk.iso: build/livedisk.bin.gz mkisofs -o $@ -b isolinux/isolinux.bin -c isolinux/boot.cat \ -no-emul-boot -boot-load-size 4 -boot-info-table \ build/iso/ + isohybrid $@ qemu: build/harddrive.bin $(QEMU) $(QEMUFLAGS) -drive file=$<,format=raw diff --git a/kernel/context/memory.rs b/kernel/context/memory.rs index 76bb934..73f808c 100644 --- a/kernel/context/memory.rs +++ b/kernel/context/memory.rs @@ -12,7 +12,8 @@ use arch::paging::temporary_page::TemporaryPage; pub struct Grant { start: VirtualAddress, size: usize, - flags: EntryFlags + flags: EntryFlags, + mapped: bool } impl Grant { @@ -36,7 +37,8 @@ impl Grant { Grant { start: to, size: size, - flags: flags + flags: flags, + mapped: true } } @@ -64,7 +66,8 @@ impl Grant { Grant { start: to, size: size, - flags: flags + flags: flags, + mapped: true } } @@ -80,7 +83,9 @@ impl Grant { self.flags } - pub fn unmap(self) { + pub fn unmap(mut self) { + assert!(self.mapped); + let mut active_table = unsafe { ActivePageTable::new() }; let mut flush_all = false; @@ -95,9 +100,13 @@ impl Grant { if flush_all { active_table.flush_all(); } + + self.mapped = false; } - pub fn unmap_inactive(self, new_table: &mut InactivePageTable, temporary_page: &mut TemporaryPage) { + pub fn unmap_inactive(mut self, new_table: &mut InactivePageTable, temporary_page: &mut TemporaryPage) { + assert!(self.mapped); + let mut active_table = unsafe { ActivePageTable::new() }; active_table.with(new_table, temporary_page, |mapper| { @@ -107,6 +116,14 @@ impl Grant { mapper.unmap_return(page); } }); + + self.mapped = false; + } +} + +impl Drop for Grant { + fn drop(&mut self) { + assert!(!self.mapped); } } diff --git a/kernel/scheme/user.rs b/kernel/scheme/user.rs index 589a8c9..03f1d4e 100644 --- a/kernel/scheme/user.rs +++ b/kernel/scheme/user.rs @@ -106,6 +106,10 @@ impl UserInner { for i in 0 .. grants.len() { let start = grants[i].start_address().get(); if to_address + full_size < start { + { + let name = context.name.lock(); + println!("{}: {}: Can grant {:X} to {:X} size {}", context.id.into(), ::core::str::from_utf8(&name).unwrap_or(""), from_address, to_address, full_size); + } grants.insert(i, Grant::map_inactive( VirtualAddress::new(from_address), VirtualAddress::new(to_address), @@ -123,6 +127,10 @@ impl UserInner { } } + { + let name = context.name.lock(); + println!("{}: {}: Can grant {:X} to {:X} size {}", context.id.into(), ::core::str::from_utf8(&name).unwrap_or(""), from_address, to_address, full_size); + } grants.push(Grant::map_inactive( VirtualAddress::new(from_address), VirtualAddress::new(to_address), diff --git a/kernel/syscall/process.rs b/kernel/syscall/process.rs index f6b4b1e..d784acf 100644 --- a/kernel/syscall/process.rs +++ b/kernel/syscall/process.rs @@ -511,10 +511,22 @@ pub fn exec(path: &[u8], arg_ptrs: &[[usize; 2]]) -> Result { // Unmap previous image, heap, grants, stack, and tls context.image.clear(); drop(context.heap.take()); - context.grants = Arc::new(Mutex::new(Vec::new())); drop(context.stack.take()); drop(context.tls.take()); + let mut unmap_grants = Vec::new(); + // FIXME: Looks like a race condition. + // Is it possible for Arc::strong_count to return 1 to two contexts that exit at the + // same time, or return 2 to both, thus either double freeing or leaking the grants? + if Arc::strong_count(&context.grants) == 1 { + mem::swap(context.grants.lock().deref_mut(), &mut unmap_grants); + } + context.grants = Arc::new(Mutex::new(Vec::new())); + + for grant in unmap_grants { + grant.unmap(); + } + if stat.st_mode & syscall::flag::MODE_SETUID == syscall::flag::MODE_SETUID { context.euid = stat.st_uid; } @@ -753,7 +765,10 @@ pub fn exit(status: usize) -> ! { let mut close_files = Vec::new(); let (pid, ppid) = { let mut context = context_lock.write(); - if Arc::strong_count(&context.files) == 1 { // FIXME: Looks like a race condition. + // FIXME: Looks like a race condition. + // Is it possible for Arc::strong_count to return 1 to two contexts that exit at the + // same time, or return 2 to both, thus either double closing or leaking the files? + if Arc::strong_count(&context.files) == 1 { mem::swap(context.files.lock().deref_mut(), &mut close_files); } context.files = Arc::new(Mutex::new(Vec::new())); @@ -796,8 +811,20 @@ pub fn exit(status: usize) -> ! { drop(context.heap.take()); drop(context.stack.take()); drop(context.tls.take()); + + let mut unmap_grants = Vec::new(); + // FIXME: Looks like a race condition. + // Is it possible for Arc::strong_count to return 1 to two contexts that exit at the + // same time, or return 2 to both, thus either double freeing or leaking the grants? + if Arc::strong_count(&context.grants) == 1 { + mem::swap(context.grants.lock().deref_mut(), &mut unmap_grants); + } context.grants = Arc::new(Mutex::new(Vec::new())); + for grant in unmap_grants { + grant.unmap(); + } + let vfork = context.vfork; context.vfork = false;