Port Futex improvements to our fork

kernel/Cargo.toml

@@ -34,6 +34,7 @@ aster-bigtcp = { path = "libs/aster-bigtcp" }
 atomic-integer-wrapper = { path = "libs/atomic-integer-wrapper" }
 id-alloc = { path = "../ostd/libs/id-alloc" }
 int-to-c-enum = { path = "libs/int-to-c-enum" }
+jhash = { path = "libs/jhash" }
 cpio-decoder = { path = "libs/cpio-decoder" }
 mariposa_data_capture = { path = "comps/mariposa_data_capture" }
 xarray = { path = "libs/xarray" }

kernel/src/context.rs

@@ -6,7 +6,7 @@ use core::{cell::Ref, mem};
 
 use aster_rights::Full;
 use ostd::{
-    mm::{Fallible, Infallible, VmReader, VmWriter},
+    mm::{Fallible, Infallible, MAX_USERSPACE_VADDR, PodAtomic, VmReader, VmWriter},
     task::{CurrentTask, Task},
 };
 
@@ -18,7 +18,7 @@ use crate::{
     },
     thread::Thread,
     util::{MultiRead, VmReaderArray},
-    vm::vmar::Vmar,
+    vm::vmar::{ROOT_VMAR_LOWEST_ADDR, Vmar},
 };
 
 /// The context that can be accessed from the current POSIX thread.
@@ -62,8 +62,7 @@ impl<'a> CurrentUserSpace<'a> {
     /// Otherwise, you can use the `current_userspace` macro
     /// to obtain an instance of `CurrentUserSpace` if it will only be used once.
     pub fn new(current_task: &'a CurrentTask) -> Self {
-        let thread_local = current_task.as_thread_local().unwrap();
-        let vmar_ref = thread_local.root_vmar().borrow();
+        let vmar_ref = current_task.as_thread_local().unwrap().root_vmar().borrow();
         Self(vmar_ref)
     }
 
@@ -78,18 +77,52 @@ impl<'a> CurrentUserSpace<'a> {
 
     /// Creates a reader to read data from the user space of the current task.
     ///
-    /// Returns `Err` if the `vaddr` and `len` do not represent a user space memory range.
+    /// Returns `Err` if `vaddr` and `len` do not represent a user space memory range.
     pub fn reader(&self, vaddr: Vaddr, len: usize) -> Result<VmReader<'_, Fallible>> {
+        // Do NOT attempt to call `check_vaddr_lowerbound` here.
+        //
+        // Linux has a **delayed buffer validation** behavior:
+        // The Linux kernel assumes that a given user-space pointer is valid until it attempts to access it.
+        // For example, the following invocation of the `read` system call with a `NULL` pointer as the buffer
+        //
+        // ```c
+        // read(fd, NULL, 1);
+        // ```
+        //
+        // will return 0 (rather than an error) if the file referred to by `fd` has zero length.
+        //
+        // Asterinas's system call entry points follow a pattern of converting user-space pointers to
+        // a reader/writer first and using the reader/writer later.
+        // So adding any pointer check here would break Asterinas's delayed buffer validation behavior.
         Ok(self.root_vmar().vm_space().reader(vaddr, len)?)
     }
 
-    /// Creates a writer to write data into the user space.
+    /// Creates a writer to write data into the user space of the current task.
     ///
-    /// Returns `Err` if the `vaddr` and `len` do not represent a user space memory range.
+    /// Returns `Err` if `vaddr` and `len` do not represent a user space memory range.
     pub fn writer(&self, vaddr: Vaddr, len: usize) -> Result<VmWriter<'_, Fallible>> {
+        // Do NOT attempt to call `check_vaddr_lowerbound` here.
+        // See the comments in the `reader` method.
         Ok(self.root_vmar().vm_space().writer(vaddr, len)?)
     }
 
+    /// Creates a reader/writer pair to read data from or write data into the user space
+    /// of the current task.
+    ///
+    /// Returns `Err` if `vaddr` and `len` do not represent a user space memory range.
+    ///
+    /// This method is semantically equivalent to calling [`Self::reader`] and [`Self::writer`]
+    /// separately, but it avoids double checking the validity of the memory region.
+    pub fn reader_writer(
+        &self,
+        vaddr: Vaddr,
+        len: usize,
+    ) -> Result<(VmReader<'_, Fallible>, VmWriter<'_, Fallible>)> {
+        // Do NOT attempt to call `check_vaddr_lowerbound` here.
+        // See the comments in the `reader` method.
+        Ok(self.root_vmar().vm_space().reader_writer(vaddr, len)?)
+    }
+
     /// Reads bytes into the destination `VmWriter` from the user space of the
     /// current process.
     ///
@@ -103,7 +136,7 @@ impl<'a> CurrentUserSpace<'a> {
         let copy_len = dest.avail();
 
         if copy_len > 0 {
-            check_vaddr(src)?;
+            check_vaddr_lowerbound(src)?;
         }
 
         let mut user_reader = self.reader(src, copy_len)?;
@@ -114,7 +147,7 @@ impl<'a> CurrentUserSpace<'a> {
     /// Reads a value typed `Pod` from the user space of the current process.
     pub fn read_val<T: Pod>(&self, src: Vaddr) -> Result<T> {
         if core::mem::size_of::<T>() > 0 {
-            check_vaddr(src)?;
+            check_vaddr_lowerbound(src)?;
         }
 
         let mut user_reader = self.reader(src, core::mem::size_of::<T>())?;
@@ -134,7 +167,7 @@ impl<'a> CurrentUserSpace<'a> {
         let copy_len = src.remain();
 
         if copy_len > 0 {
-            check_vaddr(dest)?;
+            check_vaddr_lowerbound(dest)?;
         }
 
         let mut user_writer = self.writer(dest, copy_len)?;
@@ -145,21 +178,79 @@ impl<'a> CurrentUserSpace<'a> {
     /// Writes `val` to the user space of the current process.
     pub fn write_val<T: Pod>(&self, dest: Vaddr, val: &T) -> Result<()> {
         if core::mem::size_of::<T>() > 0 {
-            check_vaddr(dest)?;
+            check_vaddr_lowerbound(dest)?;
         }
 
         let mut user_writer = self.writer(dest, core::mem::size_of::<T>())?;
         Ok(user_writer.write_val(val)?)
     }
 
+    /// Atomically loads a `PodAtomic` value with [`Ordering::Relaxed`] semantics.
+    ///
+    /// # Panics
+    ///
+    /// This method will panic if `vaddr` is not aligned on a `core::mem::align_of::<T>()`-byte
+    /// boundary.
+    ///
+    /// [`Ordering::Relaxed`]: core::sync::atomic::Ordering::Relaxed
+    pub fn atomic_load<T: PodAtomic>(&self, vaddr: Vaddr) -> Result<T> {
+        if core::mem::size_of::<T>() > 0 {
+            check_vaddr_lowerbound(vaddr)?;
+        }
+
+        let user_reader = self.reader(vaddr, core::mem::size_of::<T>())?;
+        Ok(user_reader.atomic_load()?)
+    }
+
+    /// Atomically updates a `PodAtomic` value with [`Ordering::Relaxed`] semantics.
+    ///
+    /// This method internally fetches the old value via [`atomic_load`], applies `op` to compute a
+    /// new value, and updates the value via [`atomic_compare_exchange`]. If the value changes
+    /// concurrently, this method will retry so the operation may be performed multiple times.
+    ///
+    /// If the update is completely successful, returns `Ok` with the old value (i.e., the value
+    /// _before_ applying `op`). Otherwise, it returns `Err`.
+    ///
+    /// # Panics
+    ///
+    /// This method will panic if `vaddr` is not aligned on a `core::mem::align_of::<T>()`-byte
+    /// boundary.
+    ///
+    /// [`Ordering::Relaxed`]: core::sync::atomic::Ordering::Relaxed
+    /// [`atomic_load`]: VmReader::atomic_load
+    /// [`atomic_compare_exchange`]: VmWriter::atomic_compare_exchange
+    pub fn atomic_fetch_update<T>(&self, vaddr: Vaddr, op: impl Fn(T) -> T) -> Result<T>
+    where
+        T: PodAtomic + Eq,
+    {
+        if core::mem::size_of::<T>() > 0 {
+            check_vaddr_lowerbound(vaddr)?;
+        }
+
+        let (reader, writer) = self.reader_writer(vaddr, core::mem::size_of::<T>())?;
+
+        let mut old_val = reader.atomic_load()?;
+        loop {
+            match writer.atomic_compare_exchange(&reader, old_val, op(old_val))? {
+                (_, true) => return Ok(old_val),
+                (cur_val, false) => old_val = cur_val,
+            }
+        }
+    }
+
     /// Reads a C string from the user space of the current process.
     /// The length of the string should not exceed `max_len`,
     /// including the final `\0` byte.
     pub fn read_cstring(&self, vaddr: Vaddr, max_len: usize) -> Result<CString> {
         if max_len > 0 {
-            check_vaddr(vaddr)?;
+            check_vaddr_lowerbound(vaddr)?;
         }
 
+        // Adjust `max_len` to ensure `vaddr + max_len` does not exceed `MAX_USERSPACE_VADDR`.
+        // If `vaddr` is outside user address space, `max_len` will be set to zero and further
+        // call to `self.reader` will return `EFAULT`.
+        let max_len = MAX_USERSPACE_VADDR.saturating_sub(vaddr).min(max_len);
+
         let mut user_reader = self.reader(vaddr, max_len)?;
         user_reader.read_cstring()
     }
@@ -306,18 +397,14 @@ const fn has_zero(value: usize) -> bool {
 /// segmentation fault.
 ///
 /// If it is not checked here, a kernel page fault will happen and we would
-/// deny the access in the page fault handler either. It may save a page fault
+/// deny the access in the page fault handler anyway. It may save a page fault
 /// in some occasions. More importantly, double page faults may not be handled
 /// quite well on some platforms.
-fn check_vaddr(va: Vaddr) -> Result<()> {
-    if va < crate::vm::vmar::ROOT_VMAR_LOWEST_ADDR {
-        Err(Error::with_message(
-            Errno::EFAULT,
-            "Bad user space pointer specified",
-        ))
-    } else {
-        Ok(())
+fn check_vaddr_lowerbound(va: Vaddr) -> Result<()> {
+    if va < ROOT_VMAR_LOWEST_ADDR {
+        return_errno_with_message!(Errno::EFAULT, "the userspace address is too small");
     }
+    Ok(())
 }
 
 /// Checks if the given address is aligned.

kernel/src/device/tdxguest/mod.rs

@@ -85,7 +85,7 @@ fn handle_get_report(arg: usize) -> Result<i32> {
     const SHARED_BIT: u8 = 51;
     const SHARED_MASK: u64 = 1u64 << SHARED_BIT;
     let current_task = ostd::task::Task::current().unwrap();
-    let user_space = CurrentUserSpace::new(&current_task);
+    let user_space = CurrentUserSpace::new(current_task.as_thread_local().unwrap());
     let user_request: TdxReportRequest = user_space.read_val(arg)?;
 
     let segment = FrameAllocOptions::new().alloc_segment(2).unwrap();

kernel/src/process/posix_thread/exit.rs

@@ -126,18 +126,19 @@ fn wake_clear_ctid(thread_local: &ThreadLocal) {
 ///
 /// This corresponds to Linux's `exit_robust_list`. Errors are silently ignored.
 fn wake_robust_list(thread_local: &ThreadLocal, tid: Tid) {
-    let mut robust_list = thread_local.robust_list().borrow_mut();
-
-    let list_head = match *robust_list {
+    let list_head = match thread_local.robust_list().borrow_mut().take() {
         Some(robust_list_head) => robust_list_head,
         None => return,
     };
 
-    trace!("exit: wake up the rubust list: {:?}", list_head);
+    trace!("exit: wake up the robust list: {:?}", list_head);
     for futex_addr in list_head.futexes() {
-        let _ = wake_robust_futex(futex_addr, tid)
-            .inspect_err(|err| debug!("exit: cannot wake up the robust futex: {:?}", err));
+        if let Err(err) = wake_robust_futex(futex_addr, tid) {
+            debug!(
+                "exit: cannot wake the robust futex at {:#x}: {:?}",
+                futex_addr, err
+            );
+            return;
+        }
     }
-
-    *robust_list = None;
 }