From 095c24067298edfe805ed954fabcb690f73fc9a3 Mon Sep 17 00:00:00 2001 From: wuzheng Date: Sat, 13 Apr 2024 00:24:33 +0800 Subject: [PATCH] fix bugs in mmap and syscall handler --- api/ruxos_posix_api/src/imp/fs.rs | 8 +- api/ruxos_posix_api/src/imp/mmap/api.rs | 40 ++++--- api/ruxos_posix_api/src/imp/mmap/trap.rs | 37 +++--- api/ruxos_posix_api/src/imp/mmap/utils.rs | 133 +++++++++++++--------- modules/ruxconfig/defconfig.toml | 4 +- modules/ruxhal/src/arch/aarch64/trap.rs | 6 +- 6 files changed, 131 insertions(+), 97 deletions(-) diff --git a/api/ruxos_posix_api/src/imp/fs.rs b/api/ruxos_posix_api/src/imp/fs.rs index 46338c10a..9d47b0624 100644 --- a/api/ruxos_posix_api/src/imp/fs.rs +++ b/api/ruxos_posix_api/src/imp/fs.rs @@ -23,21 +23,21 @@ use crate::{ctypes, utils::char_ptr_to_str}; use alloc::vec::Vec; pub struct File { - inner: Mutex, + pub(crate) inner: Mutex, } impl File { - fn new(inner: ruxfs::fops::File) -> Self { + pub(crate) fn new(inner: ruxfs::fops::File) -> Self { Self { inner: Mutex::new(inner), } } - fn add_to_fd_table(self) -> LinuxResult { + pub(crate) fn add_to_fd_table(self) -> LinuxResult { super::fd_ops::add_file_like(Arc::new(self)) } - fn from_fd(fd: c_int) -> LinuxResult> { + pub(crate) fn from_fd(fd: c_int) -> LinuxResult> { let f = super::fd_ops::get_file_like(fd)?; f.into_any() .downcast::() diff --git a/api/ruxos_posix_api/src/imp/mmap/api.rs b/api/ruxos_posix_api/src/imp/mmap/api.rs index 9b6ce2212..97b76c22a 100644 --- a/api/ruxos_posix_api/src/imp/mmap/api.rs +++ b/api/ruxos_posix_api/src/imp/mmap/api.rs @@ -23,9 +23,10 @@ use super::utils::{ }; #[cfg(feature = "fs")] -use super::utils::release_pages_swaped; -#[cfg(feature = "fs")] -use crate::imp::fs::sys_pwrite64; +use { + super::utils::{release_pages_swaped, write_into}, + alloc::sync::Arc, +}; /// Creates a new mapping in the virtual address space of the calling process. /// @@ -296,17 +297,10 @@ pub fn sys_msync(start: *mut c_void, len: ctypes::size_t, flags: c_int) -> c_int if !VirtAddr::from(start).is_aligned(PAGE_SIZE_4K) || len == 0 { return Err(LinuxError::EINVAL); } - for (&vaddr, &page_info) in MEM_MAP.lock().range(start..end) { - if let Some((fid, offset, size)) = page_info { - let src = vaddr as *mut c_void; - let ret_size = sys_pwrite64(fid, src, size, offset as i64) as usize; - if size != ret_size { - error!( - "sys_msync: try to pwrite(fid=0x{:x?}, size=0x{:x?}, offset=0x{:x?}) but get ret = 0x{:x?}", - fid, size, offset, ret_size - ); - return Err(LinuxError::EFAULT); - } + for (&vaddr, page_info) in MEM_MAP.lock().range(start..end) { + if let Some((file, offset, size)) = page_info { + let src = vaddr as *mut u8; + write_into(file, src, *offset as u64, *size); } } } @@ -358,13 +352,25 @@ pub fn sys_mremap( } // make sure of consistent_vma is continuous and consistent in both flags and prots. if let Some(ref mut inner_vma) = consistent_vma { - let end_offset = inner_vma.offset + (inner_vma.end_addr - inner_vma.start_addr); if inner_vma.end_addr == vma.start_addr && inner_vma.flags == vma.flags && inner_vma.prot == vma.prot - && inner_vma.fid == vma.fid - && (end_offset == vma.offset || inner_vma.fid < 0) { + #[cfg(feature = "fs")] + if inner_vma.file.is_some() { + if vma.file.is_none() { + return Err(LinuxError::EFAULT); + } + let end_offset = + inner_vma.offset + (inner_vma.end_addr - inner_vma.start_addr); + let vma_file = vma.file.as_ref().unwrap(); + let inner_file = inner_vma.file.as_ref().unwrap(); + if !Arc::ptr_eq(vma_file, inner_file) || end_offset != vma.offset { + return Err(LinuxError::EFAULT); + } + } else if vma.file.is_some() { + return Err(LinuxError::EFAULT); + } inner_vma.end_addr = vma.end_addr; } else { return Err(LinuxError::EFAULT); diff --git a/api/ruxos_posix_api/src/imp/mmap/trap.rs b/api/ruxos_posix_api/src/imp/mmap/trap.rs index ad86e781d..5b0076be6 100644 --- a/api/ruxos_posix_api/src/imp/mmap/trap.rs +++ b/api/ruxos_posix_api/src/imp/mmap/trap.rs @@ -7,18 +7,16 @@ * See the Mulan PSL v2 for more details. */ -use crate::ctypes; - -#[cfg(not(feature = "fs"))] -use ruxhal::paging::alloc_page_preload; #[cfg(feature = "fs")] -use { - crate::imp::fs::sys_pread64, - crate::imp::mmap::utils::{preload_page_with_swap, BITMAP_FREE, SWAPED_MAP, SWAP_FID}, +use crate::{ + ctypes, + imp::mmap::utils::{preload_page_with_swap, read_from, BITMAP_FREE, SWAPED_MAP, SWAP_FILE}, }; +#[cfg(not(feature = "fs"))] +use ruxhal::paging::alloc_page_preload; use crate::imp::mmap::utils::{get_mflags_from_usize, MEM_MAP, VMA_MAP}; -use core::{cmp::min, ffi::c_void, ops::Bound}; +use core::{cmp::min, ops::Bound}; use memory_addr::PAGE_SIZE_4K; use page_table::MappingFlags; use ruxhal::{ @@ -91,15 +89,15 @@ impl ruxhal::trap::TrapHandler for TrapHandlerImpl { preload_page_with_swap(&mut memory_map, &mut swaped_map, &mut off_pool); // Fill target data to assigned physical addresses, from file or zero according to mapping type - let dst = fake_vaddr.as_mut_ptr() as *mut c_void; + let dst = fake_vaddr.as_mut_ptr(); #[cfg(feature = "fs")] { if let Some(off) = swaped_map.remove(&vaddr) { off_pool.push(off); - sys_pread64(*SWAP_FID, dst, size, off as i64); - } else if vma.fid > 0 && !map_flag.is_empty() { - let off = (vma.offset + (vaddr - vma.start_addr)) as i64; - sys_pread64(vma.fid, dst, size, off); + read_from(&SWAP_FILE, dst, off as u64, size); + } else if let Some(file) = &vma.file { + let off = (vma.offset + (vaddr - vma.start_addr)) as u64; + read_from(file, dst, off, size); } else { // Set page to 0 for anonymous mapping // @@ -121,16 +119,22 @@ impl ruxhal::trap::TrapHandler for TrapHandlerImpl { } // Insert the record into `MEM_MAP` with write-back information(`None` if no need to write-back). + #[cfg(feature = "fs")] if (vma.prot & ctypes::PROT_WRITE != 0) && (vma.flags & ctypes::MAP_PRIVATE == 0) - && (vma.fid > 0) + && (vma.file.is_some()) { let map_length = min(PAGE_SIZE_4K, vma.end_addr - vaddr); let offset = vma.offset + (vaddr - vma.start_addr); - memory_map.insert(vaddr, Some((vma.fid, offset, map_length))); + memory_map.insert( + vaddr, + Some((vma.file.as_ref().unwrap().clone(), offset, map_length)), + ); } else { memory_map.insert(vaddr, None); } + #[cfg(not(feature = "fs"))] + memory_map.insert(vaddr, None); // Do actual mmapping for target vaddr // @@ -140,9 +144,6 @@ impl ruxhal::trap::TrapHandler for TrapHandlerImpl { Err(_) => false, } } else { - for mapped in vma_map.iter() { - warn!("0x{:x?}", mapped); - } warn!("vaddr=0x{:x?},cause=0x{:x?}", vaddr, cause); false } diff --git a/api/ruxos_posix_api/src/imp/mmap/utils.rs b/api/ruxos_posix_api/src/imp/mmap/utils.rs index 01f05c955..ab97573f6 100644 --- a/api/ruxos_posix_api/src/imp/mmap/utils.rs +++ b/api/ruxos_posix_api/src/imp/mmap/utils.rs @@ -10,11 +10,7 @@ use crate::ctypes; #[cfg(feature = "fs")] -use { - crate::imp::fs::{sys_open, sys_pread64, sys_pwrite64}, - core::ffi::{c_char, c_void}, - page_table::PagingError, -}; +use {crate::imp::fs::File, alloc::sync::Arc, page_table::PagingError, ruxfs::fops::OpenOptions}; use alloc::{collections::BTreeMap, vec::Vec}; use axsync::Mutex; @@ -49,7 +45,7 @@ used_fs! { pub(crate) const SWAP_PATH: &str = "swap.raw\0"; pub(crate) static SWAPED_MAP: Mutex> = Mutex::new(BTreeMap::new()); // Vaddr => (page_size, offset_at_swaped) lazy_static::lazy_static! { - pub(crate) static ref SWAP_FID: i32 = sys_open(SWAP_PATH.as_ptr() as *const c_char, (ctypes::O_RDWR| ctypes::O_TRUNC| ctypes::O_SYNC) as i32, 0); + pub(crate) static ref SWAP_FILE: Arc = open_swap_file(SWAP_PATH); pub(crate) static ref BITMAP_FREE: Mutex> = Mutex::new((0..SWAP_MAX).step_by(PAGE_SIZE_4K).collect()); } } @@ -57,17 +53,20 @@ used_fs! { pub(crate) static VMA_MAP: Mutex> = Mutex::new(BTreeMap::new()); // start_addr pub(crate) static MEM_MAP: Mutex> = Mutex::new(BTreeMap::new()); // Vaddr => (fid, offset, page_size) -type PageInfo = Option<(Fid, Offset, Len)>; // (fid, offset, page_size) +#[cfg(feature = "fs")] +type PageInfo = Option<(Arc, Offset, Len)>; // (fid, offset, page_size) +#[cfg(not(feature = "fs"))] +type PageInfo = Option; // (fid, offset, page_size) +#[cfg(feature = "fs")] type Offset = usize; -type Fid = i32; type Len = usize; /// Data structure for mapping [start_addr, end_addr) with meta data. -#[derive(Debug)] pub(crate) struct Vma { pub start_addr: usize, pub end_addr: usize, - pub fid: i32, + #[cfg(feature = "fs")] + pub file: Option>, pub offset: usize, pub prot: u32, pub flags: u32, @@ -75,22 +74,30 @@ pub(crate) struct Vma { /// Impl for Vma. impl Vma { - pub fn new(fid: i32, offset: usize, prot: u32, flags: u32) -> Self { + pub(crate) fn new(_fid: i32, offset: usize, prot: u32, flags: u32) -> Self { + #[cfg(feature = "fs")] + let file = if _fid < 0 { + None + } else { + Some(File::from_fd(_fid).expect("should be effective fid")) + }; Vma { start_addr: 0, end_addr: 0, - fid, + #[cfg(feature = "fs")] + file, offset, flags, prot, } } - pub fn clone_from(vma: &Vma, start_addr: usize, end_addr: usize) -> Self { + pub(crate) fn clone_from(vma: &Vma, start_addr: usize, end_addr: usize) -> Self { Vma { start_addr, end_addr, - fid: vma.fid, + #[cfg(feature = "fs")] + file: vma.file.clone(), offset: vma.offset, prot: vma.prot, flags: vma.prot, @@ -98,6 +105,47 @@ impl Vma { } } +/// open target file +#[cfg(feature = "fs")] +fn open_swap_file(filename: &str) -> Arc { + let mut opt = OpenOptions::new(); + opt.read(true); + opt.write(true); + opt.append(true); + opt.create(true); + + let file = ruxfs::fops::File::open(filename, &opt).expect("create swap file failed"); + Arc::new(File::new(file)) +} + +/// read from target file +#[cfg(feature = "fs")] +pub(crate) fn read_from(file: &Arc, buf: *mut u8, offset: u64, len: usize) { + let src = unsafe { core::slice::from_raw_parts_mut(buf, len) }; + let actual_len = file + .inner + .lock() + .read_at(offset, src) + .expect("read_from failed"); + if len != actual_len { + warn!("read_from len=0x{len:x} but actual_len=0x{actual_len:x}"); + } +} + +/// write into target file +#[cfg(feature = "fs")] +pub(crate) fn write_into(file: &Arc, buf: *mut u8, offset: u64, len: usize) { + let src = unsafe { core::slice::from_raw_parts_mut(buf, len) }; + let actual_len = file + .inner + .lock() + .write_at(offset, src) + .expect("write_into failed"); + if len != actual_len { + warn!("write_into len=0x{len:x} but actual_len=0x{actual_len:x}"); + } +} + /// transform usize-like mmap flags to MappingFlags pub(crate) fn get_mflags_from_usize(prot: u32) -> MappingFlags { let mut mmap_prot = MappingFlags::empty(); @@ -188,7 +236,7 @@ pub(crate) fn snatch_fixed_region( let end = start + len; // Return None if the specified address can't be used - if start >= VMA_START && end <= VMA_END { + if start < VMA_START || end > VMA_END { return None; } @@ -238,12 +286,11 @@ pub(crate) fn snatch_fixed_region( pub(crate) fn release_pages_mapped(start: usize, end: usize) { let mut memory_map = MEM_MAP.lock(); let mut removing_vaddr = Vec::new(); - for (&vaddr, &_page_info) in memory_map.range(start..end) { + for (&vaddr, _page_info) in memory_map.range(start..end) { #[cfg(feature = "fs")] - if let Some((fid, offset, size)) = _page_info { - let src = vaddr as *mut c_void; - let offset = offset as i64; - sys_pwrite64(fid, src, size, offset); + if let Some((file, offset, size)) = _page_info { + let src = vaddr as *mut u8; + write_into(file, src, *offset as u64, *size); } if pte_unmap_page(VirtAddr::from(vaddr)).is_err() { panic!("Release page failed when munmapping!"); @@ -282,8 +329,8 @@ pub(crate) fn shift_mapped_page(start: usize, end: usize, vma_offset: usize, cop } let mut opt_buffer = Vec::new(); - for (&start, &page_info) in memory_map.range(start..end) { - opt_buffer.push((start, page_info)); + for (&start, page_info) in memory_map.range(start..end) { + opt_buffer.push((start, page_info.clone())); } for (start, page_info) in opt_buffer { // opt for the PTE. @@ -315,18 +362,13 @@ pub(crate) fn shift_mapped_page(start: usize, end: usize, vma_offset: usize, cop { used_fs! { let offset = swaped_map.get(&start).unwrap(); - sys_pread64( - *SWAP_FID, - dst.as_mut_ptr() as *mut c_void, - PAGE_SIZE_4K, - *offset as i64, - ); + read_from(&SWAP_FILE, start as *mut u8, *offset as u64, PAGE_SIZE_4K); } } (fake_vaddr, flags) }; do_pte_map(VirtAddr::from(start + vma_offset), fake_vaddr, flags).unwrap(); - memory_map.insert(start + vma_offset, page_info); + memory_map.insert(start + vma_offset, page_info.clone()); } used_fs! { @@ -343,18 +385,8 @@ pub(crate) fn shift_mapped_page(start: usize, end: usize, vma_offset: usize, cop .pop() .expect("There are no free space in swap-file!"); let mut rw_buffer: [u8; PAGE_SIZE_4K] = [0_u8; PAGE_SIZE_4K]; - sys_pread64( - *SWAP_FID, - rw_buffer.as_mut_ptr() as *mut c_void, - PAGE_SIZE_4K, - off_in_swap as i64, - ); - sys_pwrite64( - *SWAP_FID, - rw_buffer.as_mut_ptr() as *mut c_void, - PAGE_SIZE_4K, - off_ptr as i64, - ); + read_from(&SWAP_FILE, rw_buffer.as_mut_ptr(), off_in_swap as u64, PAGE_SIZE_4K); + write_into(&SWAP_FILE, rw_buffer.as_mut_ptr(), off_ptr as u64, PAGE_SIZE_4K); } swaped_map.insert(start + vma_offset, off_in_swap); } @@ -376,31 +408,24 @@ pub(crate) fn preload_page_with_swap( #[cfg(feature = "fs")] Err(PagingError::NoMemory) => match memory_map.pop_first() { // For file mapping, the mapped content will be written directly to the original file. - Some((vaddr_swapped, Some((fid, offset, size)))) => { + Some((vaddr_swapped, Some((file, offset, size)))) => { let offset = offset.try_into().unwrap(); - sys_pwrite64(fid, vaddr_swapped as *mut c_void, size, offset); + write_into(&file, vaddr_swapped as *mut u8, offset, size); pte_swap_preload(VirtAddr::from(vaddr_swapped)).unwrap() } // For anonymous mapping, you need to save the mapped memory to the prepared swap file, // and record the memory address and its offset in the swap file. Some((vaddr_swapped, None)) => { let offset_get = off_pool.pop(); - if SWAP_FID.is_negative() || offset_get.is_none() { - panic!( - "No free memory for mmap or swap fid, swap fid={}", - *SWAP_FID - ); - } let offset = offset_get.unwrap(); swaped_map.insert(vaddr_swapped, offset); - sys_pwrite64( - *SWAP_FID, - vaddr_swapped as *mut c_void, + write_into( + &SWAP_FILE, + vaddr_swapped as *mut u8, + offset as u64, PAGE_SIZE_4K, - offset as i64, ); - pte_swap_preload(VirtAddr::from(vaddr_swapped)).unwrap() } _ => panic!("No memory for mmap, check if huge memory leaky exists"), diff --git a/modules/ruxconfig/defconfig.toml b/modules/ruxconfig/defconfig.toml index 60ef682db..bda9cc8df 100644 --- a/modules/ruxconfig/defconfig.toml +++ b/modules/ruxconfig/defconfig.toml @@ -31,8 +31,8 @@ pci-ranges = [] timer-frequency = "0" # Stack size of each task. -task-stack-size = "0x40000" # 256 K -# task-stack-size = "0x80000" # 512 K +# task-stack-size = "0x40000" # 256 K +task-stack-size = "0x80000" # 512 K # Number of timer ticks per second (Hz). A timer tick may contain several timer # interrupts. diff --git a/modules/ruxhal/src/arch/aarch64/trap.rs b/modules/ruxhal/src/arch/aarch64/trap.rs index 431434f3a..42597ffbc 100644 --- a/modules/ruxhal/src/arch/aarch64/trap.rs +++ b/modules/ruxhal/src/arch/aarch64/trap.rs @@ -9,8 +9,8 @@ use core::arch::global_asm; -#[cfg(feature = "irq")] -use crate::arch::enable_irqs; +#[cfg(all(feature = "irq", feature = "musl"))] +use crate::arch::{disable_irqs, enable_irqs}; #[cfg(feature = "paging")] use crate::trap::PageFaultCause; use aarch64_cpu::registers::{ESR_EL1, FAR_EL1}; @@ -74,6 +74,8 @@ fn handle_sync_exception(tf: &mut TrapFrame) { ], ); tf.r[0] = result as u64; + #[cfg(feature = "irq")] + disable_irqs(); } Some(ESR_EL1::EC::Value::DataAbortLowerEL) | Some(ESR_EL1::EC::Value::InstrAbortLowerEL) => {