From fec8f4aa0c2f0eb1de14603c38646b80dde7d3bc Mon Sep 17 00:00:00 2001
From: 4lDO2 <4lDO2@protonmail.com>
Date: Wed, 3 Feb 2021 17:37:09 +0100
Subject: [PATCH] Use physical addresses internally for futexes.
This solves a bug, that allows processes in different address spaces to
be the target of a futex wakeup call, even though that process is in
another address space!
---
src/syscall/futex.rs | 49 +++++++++++++++++++++++++++++++++++---------
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/src/syscall/futex.rs b/src/syscall/futex.rs
index ee50c433..9e04feb1 100644
--- a/src/syscall/futex.rs
+++ b/src/syscall/futex.rs
@@ -9,12 +9,19 @@ use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard};
use crate::context::{self, Context};
use crate::time;
+use crate::memory::PhysicalAddress;
+use crate::paging::{ActivePageTable, VirtualAddress};
use crate::syscall::data::TimeSpec;
-use crate::syscall::error::{Error, Result, ESRCH, EAGAIN, EINVAL};
+use crate::syscall::error::{Error, Result, ESRCH, EAGAIN, EFAULT, EINVAL};
use crate::syscall::flag::{FUTEX_WAIT, FUTEX_WAKE, FUTEX_REQUEUE};
use crate::syscall::validate::{validate_slice, validate_slice_mut};
-type FutexList = VecDeque<(usize, Arc<RwLock<Context>>)>;
+type FutexList = VecDeque<FutexEntry>;
+
+pub struct FutexEntry {
+ target_physaddr: PhysicalAddress,
+ context_lock: Arc<RwLock<Context>>,
+}
/// Fast userspace mutex list
static FUTEXES: Once<RwLock<FutexList>> = Once::new();
@@ -34,7 +41,17 @@ pub fn futexes_mut() -> RwLockWriteGuard<'static, FutexList> {
FUTEXES.call_once(init_futexes).write()
}
+// FIXME: Don't take a mutable reference to the addr, since rustc can make assumptions that the
+// pointee cannot be changed by another thread, which could make atomic ops useless.
pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32) -> Result<usize> {
+ let target_physaddr = unsafe {
+ let mut active_table = ActivePageTable::new();
+ let virtual_address = VirtualAddress::new(addr as *mut i32 as usize);
+
+ // FIXME: Already validated in syscall/mod.rs
+ active_table.translate(virtual_address).ok_or(Error::new(EFAULT))?
+ };
+
match op {
FUTEX_WAIT => {
let timeout_opt = if val2 != 0 {
@@ -69,7 +86,10 @@ pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32)
context.block("futex");
}
- futexes.push_back((addr as *mut i32 as usize, context_lock));
+ futexes.push_back(FutexEntry {
+ target_physaddr,
+ context_lock,
+ });
}
unsafe { context::switch(); }
@@ -97,9 +117,10 @@ pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32)
let mut i = 0;
while i < futexes.len() && (woken as i32) < val {
- if futexes[i].0 == addr as *mut i32 as usize {
+ if futexes[i].target_physaddr == target_physaddr {
if let Some(futex) = futexes.swap_remove_back(i) {
- futex.1.write().unblock();
+ let mut context_guard = futex.context_lock.write();
+ context_guard.unblock();
woken += 1;
}
} else {
@@ -111,7 +132,15 @@ pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32)
Ok(woken)
},
FUTEX_REQUEUE => {
- let addr2_safe = validate_slice_mut(addr2, 1).map(|addr2_safe| &mut addr2_safe[0])?;
+ let addr2_physaddr = unsafe {
+ let mut active_table = ActivePageTable::new();
+
+ let addr2_safe = validate_slice_mut(addr2, 1).map(|addr2_safe| &mut addr2_safe[0])?;
+ let addr2_virt = VirtualAddress::new(addr2_safe as *mut i32 as usize);
+
+ // FIXME: Already validated.
+ active_table.translate(addr2_virt).ok_or(Error::new(EFAULT))?
+ };
let mut woken = 0;
let mut requeued = 0;
@@ -121,9 +150,9 @@ pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32)
let mut i = 0;
while i < futexes.len() && (woken as i32) < val {
- if futexes[i].0 == addr as *mut i32 as usize {
+ if futexes[i].target_physaddr == target_physaddr {
if let Some(futex) = futexes.swap_remove_back(i) {
- futex.1.write().unblock();
+ futex.context_lock.write().unblock();
woken += 1;
}
} else {
@@ -131,8 +160,8 @@ pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32)
}
}
while i < futexes.len() && requeued < val2 {
- if futexes[i].0 == addr as *mut i32 as usize {
- futexes[i].0 = addr2_safe as *mut i32 as usize;
+ if futexes[i].target_physaddr == target_physaddr {
+ futexes[i].target_physaddr = addr2_physaddr;
requeued += 1;
}
i += 1;
--
GitLab