diff --git a/src/scheme/mod.rs b/src/scheme/mod.rs index 4e89661..6f66a44 100644 --- a/src/scheme/mod.rs +++ b/src/scheme/mod.rs @@ -302,11 +302,6 @@ pub fn schemes_mut() -> RwLockWriteGuard<'static, SchemeList> { #[allow(unused_variables)] pub trait KernelScheme: Scheme + Send + Sync + 'static { - fn kfmap(&self, number: usize, map: &syscall::data::Map, target_context: &Arc>) -> Result { - log::error!("Returning ENOSYS since kfmap can only be called on UserScheme schemes"); - Err(Error::new(ENOSYS)) - } - fn as_filetable(&self, number: usize) -> Result>>>> { Err(Error::new(EBADF)) } diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index b4117d4..05da3cf 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -120,7 +120,20 @@ enum Operation { Filetable { filetable: Arc>>> }, AddrSpace { addrspace: Arc> }, CurrentAddrSpace, + + // "operations CAN change". The reason we split changing the address space into two handle + // types, is that we would rather want the actual switch to occur when closing, as opposed to + // when writing. This is so that we can actually guarantee that no file descriptors are leaked. + AwaitingAddrSpaceChange { + new: Arc>, + new_sp: usize, + new_ip: usize, + }, + CurrentFiletable, + + AwaitingFiletableChange(Arc>>>), + // TODO: Remove this once openat is implemented, or allow openat-via-dup via e.g. the top-level // directory. OpenViaDup, @@ -530,8 +543,7 @@ impl Scheme for ProcScheme { // TODO: Define a struct somewhere? const RECORD_SIZE: usize = mem::size_of::() * 4; - let start = core::cmp::min(buf.len(), *offset); - let records = buf[start..].array_chunks_mut::(); + let records = buf.array_chunks_mut::(); let addrspace = addrspace.read(); let mut bytes_read = 0; @@ -682,8 +694,7 @@ impl Scheme for ProcScheme { // TODO: Find a better way to switch address spaces, since they also require switching // the instruction and stack pointer. Maybe remove `/regs` altogether and replace it // with `/ctx` - Operation::CurrentAddrSpace | Operation::CurrentFiletable => return Err(Error::new(EBADF)), - Operation::OpenViaDup | Operation::GrantHandle { .. } => return Err(Error::new(EBADF)), + _ => return Err(Error::new(EBADF)), } } @@ -996,12 +1007,8 @@ impl Scheme for ProcScheme { let mut filetable = hopefully_this_scheme.as_filetable(number)?; - let stopper = if info.pid == context::context_id() { with_context_mut } else { try_stop_context }; + self.handles.write().get_mut(&id).ok_or(Error::new(EBADF))?.info.operation = Operation::AwaitingFiletableChange(filetable); - stopper(info.pid, |context: &mut Context| { - context.files = filetable; - Ok(()) - })?; Ok(mem::size_of::()) } Operation::CurrentAddrSpace { .. } => { @@ -1013,25 +1020,11 @@ impl Scheme for ProcScheme { let (hopefully_this_scheme, number) = extract_scheme_number(addrspace_fd)?; let space = hopefully_this_scheme.as_addrspace(number)?; - let callback = |context: &mut Context| unsafe { - if let Some(saved_regs) = ptrace::regs_for_mut(context) { - saved_regs.iret.rip = ip; - saved_regs.iret.rsp = sp; - } else { - context.clone_entry = Some([ip, sp]); - } + self.handles.write().get_mut(&id).ok_or(Error::new(EBADF))?.info.operation = Operation::AwaitingAddrSpaceChange { new: space, new_sp: sp, new_ip: ip }; - context.set_addr_space(space); - Ok(()) - }; - if info.pid == context::context_id() { - with_context_mut(info.pid, callback)?; - } else { - try_stop_context(info.pid, callback)?; - } Ok(3 * mem::size_of::()) } - Operation::OpenViaDup | Operation::GrantHandle { .. } => return Err(Error::new(EBADF)), + _ => return Err(Error::new(EBADF)), } } @@ -1080,7 +1073,7 @@ impl Scheme for ProcScheme { Operation::CurrentFiletable => "current-filetable", Operation::OpenViaDup => "open-via-dup", - Operation::GrantHandle { .. } => return Err(Error::new(EOPNOTSUPP)), + _ => return Err(Error::new(EOPNOTSUPP)), }); read_from(buf, &path.as_bytes(), &mut 0) @@ -1111,18 +1104,38 @@ impl Scheme for ProcScheme { let mut handle = self.handles.write().remove(&id).ok_or(Error::new(EBADF))?; handle.continue_ignored_children(); - if let Operation::Trace = handle.info.operation { - ptrace::close_session(handle.info.pid); + let stop_context = if handle.info.pid == context::context_id() { with_context_mut } else { try_stop_context }; - if handle.info.flags & O_EXCL == O_EXCL { - syscall::kill(handle.info.pid, SIGKILL)?; - } + match handle.info.operation { + Operation::AwaitingAddrSpaceChange { new, new_sp, new_ip } => stop_context(handle.info.pid, |context: &mut Context| unsafe { + if let Some(saved_regs) = ptrace::regs_for_mut(context) { + saved_regs.iret.rip = new_ip; + saved_regs.iret.rsp = new_sp; + } else { + context.clone_entry = Some([new_ip, new_sp]); + } - let contexts = context::contexts(); - if let Some(context) = contexts.get(handle.info.pid) { - let mut context = context.write(); - context.ptrace_stop = false; + context.set_addr_space(new); + Ok(()) + })?, + Operation::AwaitingFiletableChange(new) => with_context_mut(handle.info.pid, |context: &mut Context| { + context.files = new; + Ok(()) + })?, + Operation::Trace => { + ptrace::close_session(handle.info.pid); + + if handle.info.flags & O_EXCL == O_EXCL { + syscall::kill(handle.info.pid, SIGKILL)?; + } + + let contexts = context::contexts(); + if let Some(context) = contexts.get(handle.info.pid) { + let mut context = context.write(); + context.ptrace_stop = false; + } } + _ => (), } Ok(0) } diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 96101db..3fce59a 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -279,9 +279,13 @@ impl UserInner { Ok(0) } - fn fmap_inner(&self, file: usize, map: &Map, context_lock: &Arc>) -> Result { + fn fmap_inner(&self, file: usize, map: &Map) -> Result { let (pid, uid, gid, context_weak, desc) = { + let context_lock = Arc::clone(context::contexts().current().ok_or(Error::new(ESRCH))?); let context = context_lock.read(); + if map.size % PAGE_SIZE != 0 { + log::warn!("Unaligned map size for context {:?}", context.name.try_read().as_deref()); + } // TODO: Faster, cleaner mechanism to get descriptor let scheme = self.scheme_id.load(Ordering::SeqCst); let mut desc_res = Err(Error::new(EBADF)); @@ -298,9 +302,8 @@ impl UserInner { } } let desc = desc_res?; - (context.id, context.euid, context.egid, Arc::downgrade(context_lock), desc) + (context.id, context.euid, context.egid, Arc::downgrade(&context_lock), desc) }; - drop(context_lock); let address = self.capture(map)?; @@ -433,7 +436,7 @@ impl Scheme for UserScheme { fn fmap(&self, file: usize, map: &Map) -> Result { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; - inner.fmap_inner(file, map, &Arc::clone(context::contexts().current().ok_or(Error::new(ESRCH))?)) + inner.fmap_inner(file, map) } fn funmap(&self, grant_address: usize, size: usize) -> Result { @@ -535,9 +538,4 @@ impl Scheme for UserScheme { inner.call(SYS_CLOSE, file, 0, 0) } } -impl crate::scheme::KernelScheme for UserScheme { - fn kfmap(&self, number: usize, map: &Map, target_context: &Arc>) -> Result { - let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; - inner.fmap_inner(number, map, target_context) - } -} +impl crate::scheme::KernelScheme for UserScheme {}