Skip to content

Commit a2bad6d

Browse files
x86_64: add copy{from,to} functions
These are safer variants for read_mut() for userspace memory accesses. Pretty documented rn but more detail could be done ig :^) Signed-off-by: Anhad Singh <[email protected]>
1 parent ca83275 commit a2bad6d

File tree

10 files changed

+199
-32
lines changed

10 files changed

+199
-32
lines changed

src/aero_kernel/src/arch/x86_64/interrupts/exceptions.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use super::{io, InterruptErrorStack};
1919

2020
use crate::arch::controlregs;
21+
use crate::arch::tls::get_percpu;
2122
use crate::mem::paging::PageFaultErrorCode;
2223

2324
use crate::unwind;
@@ -120,6 +121,12 @@ pub fn breakpoint(stack: &mut InterruptErrorStack) {
120121
}
121122

122123
pub(super) fn page_fault(stack: &mut InterruptErrorStack) {
124+
let pf_resume = get_percpu().pf_resume.as_ptr::<u8>();
125+
if !pf_resume.is_null() {
126+
stack.stack.iret.rip = pf_resume as u64;
127+
return;
128+
}
129+
123130
let accessed_address = controlregs::read_cr2();
124131
let reason = PageFaultErrorCode::from_bits_truncate(stack.code);
125132

src/aero_kernel/src/arch/x86_64/interrupts/idt.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,21 +196,21 @@ pub fn init() {
196196
INTERRUPT_HANDLERS.lock()[7] = IrqHandler::ErrorHandler(exceptions::device_not_available);
197197
INTERRUPT_HANDLERS.lock()[8] = IrqHandler::ErrorHandler(exceptions::double_fault);
198198

199-
// INTERRUPT_HANDLERS.lock()[9] is reserved.
199+
// INTERRUPT_HANDLERS[9] is reserved.
200200
INTERRUPT_HANDLERS.lock()[10] = IrqHandler::ErrorHandler(exceptions::invalid_tss);
201201
INTERRUPT_HANDLERS.lock()[11] = IrqHandler::ErrorHandler(exceptions::segment_not_present);
202202
INTERRUPT_HANDLERS.lock()[12] = IrqHandler::ErrorHandler(exceptions::stack_segment);
203203
INTERRUPT_HANDLERS.lock()[13] = IrqHandler::ErrorHandler(exceptions::protection);
204204
INTERRUPT_HANDLERS.lock()[14] = IrqHandler::ErrorHandler(exceptions::page_fault);
205205

206-
// INTERRUPT_HANDLERS.lock()[15] is reserved.
206+
// INTERRUPT_HANDLERS[15] is reserved.
207207
INTERRUPT_HANDLERS.lock()[16] = IrqHandler::ErrorHandler(exceptions::fpu_fault);
208208
INTERRUPT_HANDLERS.lock()[17] = IrqHandler::ErrorHandler(exceptions::alignment_check);
209209
INTERRUPT_HANDLERS.lock()[18] = IrqHandler::ErrorHandler(exceptions::machine_check);
210210
INTERRUPT_HANDLERS.lock()[19] = IrqHandler::ErrorHandler(exceptions::simd);
211211
INTERRUPT_HANDLERS.lock()[20] = IrqHandler::ErrorHandler(exceptions::virtualization);
212212

213-
// INTERRUPT_HANDLERS.lock()[21..29] are reserved.
213+
// INTERRUPT_HANDLERS[21..29] are reserved.
214214
INTERRUPT_HANDLERS.lock()[30] = IrqHandler::ErrorHandler(exceptions::security);
215215

216216
unsafe {

src/aero_kernel/src/arch/x86_64/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub mod syscall;
2525
pub mod task;
2626
pub mod time;
2727
pub mod tls;
28+
pub mod user_copy;
2829

2930
use core::sync::atomic::Ordering;
3031

src/aero_kernel/src/arch/x86_64/task.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,12 @@ pub fn userland_last_address() -> VirtAddr {
109109
})
110110
}
111111

112+
/// Returns whether the given pointer is within the userland address space.
113+
pub fn user_access_ok<T>(ptr: *const T) -> bool {
114+
let size = core::mem::size_of::<T>();
115+
VirtAddr::new(ptr as u64 + size as u64) <= super::task::userland_last_address()
116+
}
117+
112118
const USERLAND_STACK_SIZE: u64 = 0x64000;
113119

114120
//(1 << 47) - (Size4KiB::SIZE * 2)

src/aero_kernel/src/arch/x86_64/tls.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use alloc::vec::Vec;
3131
use super::gdt::*;
3232
use super::io;
3333

34+
use crate::mem::paging::VirtAddr;
3435
use crate::utils::sync::Mutex;
3536

3637
use raw_cpuid::FeatureInfo;
@@ -115,6 +116,7 @@ pub struct PerCpuData {
115116
pub lapic_timer_frequency: u32,
116117

117118
pub(super) gdt: &'static mut [GdtEntry],
119+
pub(super) pf_resume: VirtAddr,
118120
}
119121

120122
/// SAFETY: The GS base should point to the kernel PCR.
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (C) 2021-2023 The Aero Project Developers.
2+
//
3+
// This file is part of The Aero Project.
4+
//
5+
// Aero is free software: you can redistribute it and/or modify
6+
// it under the terms of the GNU General Public License as published by
7+
// the Free Software Foundation, either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// Aero is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU General Public License
16+
// along with Aero. If not, see <https://www.gnu.org/licenses/>.
17+
18+
use core::mem::MaybeUninit;
19+
use core::ops::{Deref, DerefMut};
20+
21+
use crate::mem::paging::VirtAddr;
22+
23+
use super::task::user_access_ok;
24+
use super::tls::get_percpu;
25+
26+
/// Copy to/from a block of data from user space. Returns whether the copy was successful.
27+
///
28+
/// # Safety
29+
/// The caller must ensure that:
30+
/// * If copying to userspace, the `dest` pointer must be within the userland address space.
31+
/// * If copying from userspace, the `dest` pointer must be valid for `size` bytes.
32+
///
33+
/// ## Concurrent Accesses
34+
/// Concurrent access, *including potential data races with userspace memory*, are permitted since
35+
/// other userspace threads or processes may modify the memory concurrently. This behavior is
36+
/// similar to how [`std::io`] permits data races with file contents on disk.
37+
///
38+
/// [`std::io`]: https://doc.rust-lang.org/std/io/index.html
39+
#[naked]
40+
unsafe extern "C" fn copy_to_from_user(
41+
dest: *mut u8,
42+
src: *const u8,
43+
size: usize,
44+
45+
fault_resume: *const u8,
46+
) -> bool {
47+
// Registers used:
48+
//
49+
// %rax = argument 1, `dest`
50+
// %rsi = argument 2, `src`
51+
// %rdx = argument 3, `size`
52+
// %rcx = argument 4, `fault_resume`
53+
asm!(
54+
// Copy `fault_resume` out of %rcx because it will be utilized by `rep movsb` latter.
55+
"mov r10, rcx",
56+
// Setup the page fault resume.
57+
"lea rax, 1f",
58+
"mov [r10], rax",
59+
// XXX: From this point until the fault return is reset, no function calls or stack
60+
// manipulations should be performed. We must ensure the ability to restore all callee
61+
// registers without any knowledge of the exact location within this code where a fault may
62+
// occur.
63+
//
64+
// Copy 8 bytes at a time and then one byte at a time for the remainder.
65+
"mov rcx, rdx",
66+
"shr rcx, 3",
67+
"rep movsq",
68+
"and edx, 7",
69+
"je 2f",
70+
"mov ecx, edx",
71+
"rep movsb",
72+
// Set return value to `true`.
73+
"2:",
74+
"mov eax, 1",
75+
// Reset the `fault_resume` pointer and return.
76+
"3:",
77+
"mov qword ptr [r10], 0",
78+
"ret",
79+
// `fault_resume` handler - set return value to `false` and return.
80+
"1:",
81+
"xor eax, eax",
82+
"jmp 3b",
83+
options(noreturn)
84+
)
85+
}
86+
87+
/// Copy a structure from userspace memory. Returns whether the copy was successful.
88+
#[must_use]
89+
fn copy_from_user<T>(dest: &mut MaybeUninit<T>, src: *const T) -> bool {
90+
let fault_resume = &get_percpu().pf_resume as *const _ as *const u8;
91+
let size = core::mem::size_of::<T>();
92+
93+
user_access_ok(src);
94+
95+
// SAFETY: We have verified that the `src` pointer is within the userland address space.
96+
unsafe { copy_to_from_user(dest.as_mut_ptr().cast(), src.cast(), size, fault_resume) }
97+
}
98+
99+
/// Copy a structure from userspace memory. Returns whether the copy was successful.
100+
#[must_use]
101+
fn copy_to_user<T>(dest: *mut T, src: &T) -> bool {
102+
let fault_resume = &get_percpu().pf_resume as *const _ as *const u8;
103+
let size = core::mem::size_of::<T>();
104+
let src_ptr = src as *const T;
105+
106+
user_access_ok(dest);
107+
108+
// SAFETY: We have verified that the `dest` pointer is within the userland address space.
109+
unsafe { copy_to_from_user(dest.cast(), src_ptr.cast(), size, fault_resume) }
110+
}
111+
112+
/// A reference to a structure in userspace memory, which can be either read-only or read-write.
113+
///
114+
/// Concurrent access, *including data races to/from userspace memory*, are permitted. See the
115+
/// documentation of [`copy_to_from_user`] for more information.
116+
pub struct UserRef<T> {
117+
ptr: *mut T,
118+
val: T,
119+
}
120+
121+
impl<T> UserRef<T> {
122+
pub unsafe fn new(address: VirtAddr) -> Self {
123+
let mut val = MaybeUninit::<T>::uninit();
124+
125+
// FIXME: Return an error if the copy fails.
126+
assert!(copy_from_user(&mut val, address.as_ptr()));
127+
128+
Self {
129+
ptr: address.as_mut_ptr(),
130+
// SAFETY: We have initialized the value via `copy_from_user` above.
131+
val: unsafe { val.assume_init() },
132+
}
133+
}
134+
}
135+
136+
impl<T> Deref for UserRef<T> {
137+
type Target = T;
138+
139+
fn deref(&self) -> &Self::Target {
140+
&self.val
141+
}
142+
}
143+
144+
impl<T> DerefMut for UserRef<T> {
145+
fn deref_mut(&mut self) -> &mut Self::Target {
146+
&mut self.val
147+
}
148+
}
149+
150+
impl<T> Drop for UserRef<T> {
151+
fn drop(&mut self) {
152+
assert!(copy_to_user(self.ptr, &self.val));
153+
}
154+
}

src/aero_kernel/src/drivers/drm/mod.rs

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use alloc::vec::Vec;
2424
use bit_field::BitField;
2525
use hashbrown::HashMap;
2626

27+
use crate::arch::user_copy::UserRef;
2728
use crate::fs;
2829
use crate::fs::inode::INodeInterface;
2930
use crate::fs::{devfs, FileSystemError};
@@ -405,7 +406,7 @@ impl INodeInterface for Drm {
405406
fn ioctl(&self, command: usize, arg: usize) -> fs::Result<usize> {
406407
match command {
407408
DRM_IOCTL_VERSION => {
408-
let struc = VirtAddr::new(arg as u64).read_mut::<DrmVersion>().unwrap();
409+
let mut struc = unsafe { UserRef::<DrmVersion>::new(VirtAddr::new(arg as u64)) };
409410

410411
let (major, minor, patch_level) = self.device.driver_version();
411412
let (name, desc, date) = self.device.driver_info();
@@ -422,7 +423,7 @@ impl INodeInterface for Drm {
422423
}
423424

424425
DRM_IOCTL_GET_CAP => {
425-
let struc = VirtAddr::new(arg as u64).read_mut::<DrmGetCap>().unwrap();
426+
let mut struc = unsafe { UserRef::<DrmGetCap>::new(VirtAddr::new(arg as u64)) };
426427

427428
// NOTE: The user is responsible for zeroing out the structure.
428429
match struc.capability {
@@ -442,9 +443,8 @@ impl INodeInterface for Drm {
442443
}
443444

444445
DRM_IOCTL_MODE_GETRESOURCES => {
445-
let struc = VirtAddr::new(arg as u64)
446-
.read_mut::<DrmModeCardRes>()
447-
.unwrap();
446+
let mut struc =
447+
unsafe { UserRef::<DrmModeCardRes>::new(VirtAddr::new(arg as u64)) };
448448

449449
/// Copies the mode object IDs into the user provided buffer. For safety, checkout
450450
/// the [`copy_field`] function.
@@ -489,15 +489,15 @@ impl INodeInterface for Drm {
489489
}
490490

491491
DRM_IOCTL_GET_CRTC => {
492-
let struc = VirtAddr::new(arg as u64).read_mut::<DrmModeCrtc>().unwrap();
492+
let struc = unsafe { UserRef::<DrmModeCrtc>::new(VirtAddr::new(arg as u64)) };
493493
let _object = self.find_object(struc.crtc_id).unwrap().as_crtc().unwrap();
494494

495495
log::warn!("drm::get_crtc: is a stub!");
496496
Ok(0)
497497
}
498498

499499
DRM_IOCTL_SET_CRTC => {
500-
let struc = VirtAddr::new(arg as u64).read_mut::<DrmModeCrtc>().unwrap();
500+
let struc = unsafe { UserRef::<DrmModeCrtc>::new(VirtAddr::new(arg as u64)) };
501501
let _object = self.find_object(struc.crtc_id).unwrap().as_crtc().unwrap();
502502

503503
let object = self
@@ -513,9 +513,8 @@ impl INodeInterface for Drm {
513513
}
514514

515515
DRM_IOCTL_GET_ENCODER => {
516-
let struc = VirtAddr::new(arg as u64)
517-
.read_mut::<DrmModeGetEncoder>()
518-
.unwrap();
516+
let mut struc =
517+
unsafe { UserRef::<DrmModeGetEncoder>::new(VirtAddr::new(arg as u64)) };
519518

520519
let object = self
521520
.find_object(struc.encoder_id)
@@ -544,9 +543,8 @@ impl INodeInterface for Drm {
544543
}
545544

546545
DRM_IOCTL_GET_CONNECTOR => {
547-
let struc = VirtAddr::new(arg as u64)
548-
.read_mut::<DrmModeGetConnector>()
549-
.unwrap();
546+
let mut struc =
547+
unsafe { UserRef::<DrmModeGetConnector>::new(VirtAddr::new(arg as u64)) };
550548

551549
let object = self
552550
.find_object(struc.connector_id)
@@ -592,9 +590,8 @@ impl INodeInterface for Drm {
592590
}
593591

594592
DRM_IOCTL_MODE_CREATE_DUMB => {
595-
let struc = VirtAddr::new(arg as u64)
596-
.read_mut::<DrmModeCreateDumb>()
597-
.unwrap();
593+
let mut struc =
594+
unsafe { UserRef::<DrmModeCreateDumb>::new(VirtAddr::new(arg as u64)) };
598595

599596
let (mut buffer, pitch) =
600597
self.device
@@ -612,9 +609,7 @@ impl INodeInterface for Drm {
612609
}
613610

614611
DRM_IOCTL_MODE_ADDFB => {
615-
let struc = VirtAddr::new(arg as u64)
616-
.read_mut::<DrmModeFbCmd>()
617-
.unwrap();
612+
let mut struc = unsafe { UserRef::<DrmModeFbCmd>::new(VirtAddr::new(arg as u64)) };
618613

619614
let handle = self.find_handle(struc.handle).unwrap();
620615
self.device
@@ -628,9 +623,8 @@ impl INodeInterface for Drm {
628623
}
629624

630625
DRM_IOCTL_MODE_MAP_DUMB => {
631-
let struc = VirtAddr::new(arg as u64)
632-
.read_mut::<DrmModeMapDumb>()
633-
.unwrap();
626+
let mut struc =
627+
unsafe { UserRef::<DrmModeMapDumb>::new(VirtAddr::new(arg as u64)) };
634628

635629
let handle = self.find_handle(struc.handle).unwrap();
636630
struc.offset = handle.mapping as _;

src/aero_kernel/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
int_roundings, // https://github.com/rust-lang/rust/issues/88581
4747
const_ptr_is_null, // https://github.com/rust-lang/rust/issues/74939
4848
trait_upcasting, // https://github.com/rust-lang/rust/issues/65991
49+
naked_functions, // https://github.com/rust-lang/rust/issues/32408
4950
)]
5051
#![deny(trivial_numeric_casts, unused_allocation)]
5152
#![test_runner(crate::tests::test_runner)]

0 commit comments

Comments
 (0)