From 62eab8a2fe9e0ccf735906bd191d2baaffd19a18 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 9 Apr 2023 12:04:08 +0200 Subject: [PATCH 1/7] Retry rather than panic if clone_entry is unset. --- src/scheme/proc.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index b90b5d9e78..2d862ca1c8 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -1358,11 +1358,16 @@ impl KernelScheme for ProcScheme { extern "C" fn clone_handler() { let context_lock = Arc::clone(context::contexts().current().expect("expected the current context to be set in a spawn closure")); - unsafe { - let [ip, sp] = context_lock.read().clone_entry.expect("clone_entry must be set"); - let [arg, is_singlestep] = [0; 2]; + loop { + unsafe { + let Some([ip, sp]) = context_lock.read().clone_entry else { + context_lock.write().status = Status::Stopped(SIGSTOP); + continue; + }; + let [arg, is_singlestep] = [0; 2]; - crate::start::usermode(ip, sp, arg, is_singlestep); + crate::start::usermode(ip, sp, arg, is_singlestep); + } } } From 6c3f577f05c064f9ea4c02c4d1d5162bb48710d5 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 30 Apr 2023 17:53:17 +0200 Subject: [PATCH 2/7] Also translate the 12-bit page offset in SYS_FUTEX. --- src/syscall/futex.rs | 118 +++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 67 deletions(-) diff --git a/src/syscall/futex.rs b/src/syscall/futex.rs index 2c195582fc..b11bc59f23 100644 --- a/src/syscall/futex.rs +++ b/src/syscall/futex.rs @@ -2,20 +2,21 @@ //! Futex or Fast Userspace Mutex is "a method for waiting until a certain condition becomes true." //! //! For more information about futexes, please read [this](https://eli.thegreenplace.net/2018/basics-of-futexes/) blog post, and the [futex(2)](http://man7.org/linux/man-pages/man2/futex.2.html) man page -use alloc::sync::Arc; use alloc::collections::VecDeque; +use alloc::sync::Arc; use core::intrinsics; -use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; +use spin::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use rmm::Arch; -use crate::context::{self, Context}; -use crate::time; +use crate::context::{self, memory::AddrSpace, Context}; use crate::memory::PhysicalAddress; -use crate::paging::VirtualAddress; +use crate::paging::{Page, VirtualAddress}; +use crate::time; + use crate::syscall::data::TimeSpec; -use crate::syscall::error::{Error, Result, ESRCH, EAGAIN, EFAULT, EINVAL}; -use crate::syscall::flag::{FUTEX_WAIT, FUTEX_WAIT64, FUTEX_WAKE, FUTEX_REQUEUE}; +use crate::syscall::error::{Error, Result, EAGAIN, EFAULT, EINVAL, ESRCH}; +use crate::syscall::flag::{FUTEX_REQUEUE, FUTEX_WAIT, FUTEX_WAIT64, FUTEX_WAKE}; use crate::syscall::validate::validate_array; type FutexList = VecDeque; @@ -25,42 +26,30 @@ pub struct FutexEntry { context_lock: Arc>, } -/// Fast userspace mutex list -static FUTEXES: Once> = Once::new(); +// TODO: Process-private futexes? In that case, put the futex table in each AddrSpace. +// TODO: Hash table? +static FUTEXES: RwLock = RwLock::new(FutexList::new()); -/// Initialize futexes, called if needed -fn init_futexes() -> RwLock { - RwLock::new(VecDeque::new()) -} +fn validate_and_translate_virt(space: &AddrSpace, addr: VirtualAddress) -> Option { + // TODO: Move this elsewhere! + if addr.data().saturating_add(core::mem::size_of::()) >= crate::USER_END_OFFSET { + return None; + } -/// Get the global futexes list, const -pub fn futexes() -> RwLockReadGuard<'static, FutexList> { - FUTEXES.call_once(init_futexes).read() -} + let page = Page::containing_address(addr); + let off = addr.data() - page.start_address().data(); -/// Get the global futexes list, mutable -pub fn futexes_mut() -> RwLockWriteGuard<'static, FutexList> { - FUTEXES.call_once(init_futexes).write() + let (frame, _) = space.table.utable.translate(page.start_address())?; + + Some(frame.add(off)) } pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> Result { let addr_space = Arc::clone(context::current()?.read().addr_space()?); - let (target_physaddr, _) = { - let virtual_address = VirtualAddress::new(addr); - - if !crate::CurrentRmmArch::virt_is_valid(virtual_address) { - return Err(Error::new(EFAULT)); - } - // TODO: Use this all over the code, making sure that no user pointers that are higher half - // can get to the page table walking procedure. - #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] - if virtual_address.data() & (1 << 63) == (1 << 63) { - return Err(Error::new(EFAULT)); - } - - addr_space.read().table.utable.translate(virtual_address).ok_or(Error::new(EFAULT))? - }; + let target_physaddr = + validate_and_translate_virt(&*addr_space.read(), VirtualAddress::new(addr)) + .ok_or(Error::new(EFAULT))?; match op { // TODO: FUTEX_WAIT_MULTIPLE? @@ -75,28 +64,31 @@ pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> R }; { - let mut futexes = futexes_mut(); + let mut futexes = FUTEXES.write(); - let context_lock = { - let contexts = context::contexts(); - let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; - Arc::clone(context_lock) - }; + let context_lock = context::current()?; - // TODO: Is the implicit SeqCst ordering too strong here? let (fetched, expected) = if op == FUTEX_WAIT { // Must be aligned, otherwise it could cross a page boundary and mess up the // (simpler) validation we did in the first place. if addr % 4 != 0 { return Err(Error::new(EINVAL)); } - (u64::from(unsafe { intrinsics::atomic_load_seqcst::(addr as *const u32) }), u64::from(val as u32)) + ( + u64::from(unsafe { + intrinsics::atomic_load_seqcst::(addr as *const u32) + }), + u64::from(val as u32), + ) } else { // op == FUTEX_WAIT64 if addr % 8 != 0 { return Err(Error::new(EINVAL)); } - (unsafe { intrinsics::atomic_load_seqcst::(addr as *const u64) }, val as u64) + ( + unsafe { intrinsics::atomic_load_seqcst::(addr as *const u64) }, + val as u64, + ) }; if fetched != expected { return Err(Error::new(EAGAIN)); @@ -107,7 +99,9 @@ pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> R if let Some(timeout) = timeout_opt { let start = time::monotonic(); - let end = start + (timeout.tv_sec as u128 * time::NANOS_PER_SEC) + (timeout.tv_nsec as u128); + let end = start + + (timeout.tv_sec as u128 * time::NANOS_PER_SEC) + + (timeout.tv_nsec as u128); context.wake = Some(end); } @@ -120,7 +114,9 @@ pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> R }); } - unsafe { context::switch(); } + unsafe { + context::switch(); + } if timeout_opt.is_some() { let context_lock = { @@ -136,12 +132,12 @@ pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> R } Ok(0) - }, + } FUTEX_WAKE => { let mut woken = 0; { - let mut futexes = futexes_mut(); + let mut futexes = FUTEXES.write(); let mut i = 0; @@ -160,29 +156,17 @@ pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> R } Ok(woken) - }, + } FUTEX_REQUEUE => { - let (addr2_physaddr, _) = { - let addr2_virt = VirtualAddress::new(addr2); - - if !crate::CurrentRmmArch::virt_is_valid(addr2_virt) { - return Err(Error::new(EFAULT)); - } - - // TODO - #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] - if addr2_virt.data() & (1 << 63) == (1 << 63) { - return Err(Error::new(EFAULT)); - } - - addr_space.read().table.utable.translate(addr2_virt).ok_or(Error::new(EFAULT))? - }; + let addr2_physaddr = + validate_and_translate_virt(&*addr_space.read(), VirtualAddress::new(addr2)) + .ok_or(Error::new(EFAULT))?; let mut woken = 0; let mut requeued = 0; { - let mut futexes = futexes_mut(); + let mut futexes = FUTEXES.write(); let mut i = 0; while i < futexes.len() && woken < val { @@ -204,7 +188,7 @@ pub fn futex(addr: usize, op: usize, val: usize, val2: usize, addr2: usize) -> R } Ok(woken) - }, - _ => Err(Error::new(EINVAL)) + } + _ => Err(Error::new(EINVAL)), } } From 45f031b50de62f6238dad0f484e7b3bbc7ed0056 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 30 Apr 2023 17:54:05 +0200 Subject: [PATCH 3/7] Improve clone_entry lock granularity. --- src/scheme/proc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index 2d862ca1c8..176eac07df 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -1360,7 +1360,7 @@ extern "C" fn clone_handler() { loop { unsafe { - let Some([ip, sp]) = context_lock.read().clone_entry else { + let Some([ip, sp]) = ({ context_lock.read().clone_entry }) else { context_lock.write().status = Status::Stopped(SIGSTOP); continue; }; From a5168b4442e29256fc91433783f21772bb452b03 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 30 Apr 2023 18:11:20 +0200 Subject: [PATCH 4/7] Fix warnings. --- src/syscall/futex.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/syscall/futex.rs b/src/syscall/futex.rs index b11bc59f23..8f7ca2fc8f 100644 --- a/src/syscall/futex.rs +++ b/src/syscall/futex.rs @@ -5,9 +5,7 @@ use alloc::collections::VecDeque; use alloc::sync::Arc; use core::intrinsics; -use spin::{RwLock, RwLockReadGuard, RwLockWriteGuard}; - -use rmm::Arch; +use spin::RwLock; use crate::context::{self, memory::AddrSpace, Context}; use crate::memory::PhysicalAddress; From 99ffc370e899d733c41ef809f134c31005842f9f Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 30 Apr 2023 18:11:34 +0200 Subject: [PATCH 5/7] Work around repr(packed) but not fixing UB. --- src/devices/uart_16550.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/devices/uart_16550.rs b/src/devices/uart_16550.rs index 36f09bd5dd..51a0b09d5c 100644 --- a/src/devices/uart_16550.rs +++ b/src/devices/uart_16550.rs @@ -1,4 +1,5 @@ use core::convert::TryInto; +use core::ptr::{addr_of, addr_of_mut}; use crate::syscall::io::{Io, Mmio, ReadOnly}; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] @@ -70,20 +71,23 @@ where T::Value: From + TryInto, { pub fn init(&mut self) { - //TODO: Cleanup - self.int_en.write(0x00.into()); - self.line_ctrl.write(0x80.into()); - self.data.write(0x01.into()); - self.int_en.write(0x00.into()); - self.line_ctrl.write(0x03.into()); - self.fifo_ctrl.write(0xC7.into()); - self.modem_ctrl.write(0x0B.into()); - self.int_en.write(0x01.into()); + unsafe { + //TODO: Cleanup + // FIXME: Fix UB if unaligned + (&mut *addr_of_mut!(self.int_en)).write(0x00.into()); + (&mut *addr_of_mut!(self.line_ctrl)).write(0x80.into()); + (&mut *addr_of_mut!(self.data)).write(0x01.into()); + (&mut *addr_of_mut!(self.int_en)).write(0x00.into()); + (&mut *addr_of_mut!(self.line_ctrl)).write(0x03.into()); + (&mut *addr_of_mut!(self.fifo_ctrl)).write(0xC7.into()); + (&mut *addr_of_mut!(self.modem_ctrl)).write(0x0B.into()); + (&mut *addr_of_mut!(self.int_en)).write(0x01.into()); + } } fn line_sts(&self) -> LineStsFlags { LineStsFlags::from_bits_truncate( - (self.line_sts.read() & 0xFF.into()) + (unsafe { &*addr_of!(self.line_sts) }.read() & 0xFF.into()) .try_into() .unwrap_or(0), ) @@ -92,7 +96,7 @@ where pub fn receive(&mut self) -> Option { if self.line_sts().contains(LineStsFlags::INPUT_FULL) { Some( - (self.data.read() & 0xFF.into()) + (unsafe { &*addr_of!(self.data) }.read() & 0xFF.into()) .try_into() .unwrap_or(0), ) @@ -103,7 +107,7 @@ where pub fn send(&mut self, data: u8) { while !self.line_sts().contains(LineStsFlags::OUTPUT_EMPTY) {} - self.data.write(data.into()) + unsafe { &mut *addr_of_mut!(self.data) }.write(data.into()) } pub fn write(&mut self, buf: &[u8]) { From c604d6b0512358113a78c2e8dc669715f0825424 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 30 Apr 2023 18:13:27 +0200 Subject: [PATCH 6/7] Make IDT #[repr(C)]. --- src/arch/x86/idt.rs | 2 +- src/arch/x86_64/idt.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/arch/x86/idt.rs b/src/arch/x86/idt.rs index bfca4d01d4..45a5e9e84e 100644 --- a/src/arch/x86/idt.rs +++ b/src/arch/x86/idt.rs @@ -27,7 +27,7 @@ pub static mut IDTR: DescriptorTablePointer = DescriptorTablePointe pub type IdtEntries = [IdtEntry; 256]; pub type IdtReservations = [AtomicU32; 8]; -#[repr(packed)] +#[repr(C)] pub struct Idt { entries: IdtEntries, reservations: IdtReservations, diff --git a/src/arch/x86_64/idt.rs b/src/arch/x86_64/idt.rs index 1377ff75fe..6063f7d2be 100644 --- a/src/arch/x86_64/idt.rs +++ b/src/arch/x86_64/idt.rs @@ -27,7 +27,7 @@ pub static mut IDTR: DescriptorTablePointer = DescriptorTablePointe pub type IdtEntries = [IdtEntry; 256]; pub type IdtReservations = [AtomicU64; 4]; -#[repr(packed)] +#[repr(C)] pub struct Idt { entries: IdtEntries, reservations: IdtReservations, From 50b877d1aab9f808b0639a7913a953161d8ab361 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 30 Apr 2023 18:13:55 +0200 Subject: [PATCH 7/7] Remove ignored unaligned_references exception. --- src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4b47c1cee5..3c03ae532b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,8 +3,6 @@ //! The Redox OS Kernel is a microkernel that supports `x86_64` systems and //! provides Unix-like syscalls for primarily Rust applications -//TODO: fix the need to generate references to packed fields -#![allow(unaligned_references)] // Useful for adding comments about different branches #![allow(clippy::if_same_then_else)] // Useful in the syscall function