From 095646517daa00e22852f84f2dbfb164aee86dca Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 12 Apr 2025 20:10:28 +0200 Subject: [PATCH 1/4] Use a single mmap + read for each segment --- redox-rt/src/proc.rs | 67 +++++--------------------------------------- 1 file changed, 7 insertions(+), 60 deletions(-) diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs index c04897fb3a..b6e1047d13 100644 --- a/redox-rt/src/proc.rs +++ b/redox-rt/src/proc.rs @@ -168,74 +168,21 @@ where total_page_count * PAGE_SIZE, flags, )?; - syscall::lseek(*image_file, segment.p_offset as isize, SEEK_SET) - .map_err(|_| Error::new(EIO))?; - // If unaligned, read the head page separately. - let (first_aligned_page, remaining_filesz) = if voff > 0 { - let bytes_to_next_page = PAGE_SIZE - voff; + // TODO: Attempt to mmap with MAP_PRIVATE directly from the image file instead. - let (_guard, dst_page) = - unsafe { MmapGuard::map_mut_anywhere(*grants_fd, vaddr, PAGE_SIZE)? }; + if filesz > 0 { + syscall::lseek(*image_file, segment.p_offset as isize, SEEK_SET) + .map_err(|_| Error::new(EIO))?; - let length = core::cmp::min(bytes_to_next_page, filesz); - - read_all(*image_file, None, &mut dst_page[voff..][..length])?; - - (vaddr + PAGE_SIZE, filesz - length) - } else { - (vaddr, filesz) - }; - - let remaining_page_count = remaining_filesz.div_floor(PAGE_SIZE); - let tail_bytes = remaining_filesz % PAGE_SIZE; - - // TODO: Unless the calling process if *very* memory-constrained, the max amount of - // pages per iteration has no limit other than the time it takes to setup page - // tables. - // - // TODO: Reserve PAGES_PER_ITER "scratch pages" of virtual memory for that type of - // situation? - const PAGES_PER_ITER: usize = 64; - - // TODO: Before this loop, attempt to mmap with MAP_PRIVATE directly from the image - // file. - - for page_idx in (0..remaining_page_count).step_by(PAGES_PER_ITER) { - // Use commented out lines to trigger kernel bug (FIXME). - - //let pages_in_this_group = core::cmp::min(PAGES_PER_ITER, file_page_count - page_idx * PAGES_PER_ITER); - let pages_in_this_group = - core::cmp::min(PAGES_PER_ITER, remaining_page_count - page_idx); - - if pages_in_this_group == 0 { - break; - } - - // TODO: MAP_FIXED to optimize away funmap? let (_guard, dst_memory) = unsafe { MmapGuard::map_mut_anywhere( *grants_fd, - first_aligned_page + page_idx * PAGE_SIZE, // offset - pages_in_this_group * PAGE_SIZE, // size + vaddr, // offset + (voff + filesz).next_multiple_of(PAGE_SIZE), // size )? }; - - // TODO: Are &mut [u8] and &mut [[u8; PAGE_SIZE]] interchangeable (if the - // lengths are aligned, obviously)? - - read_all(*image_file, None, dst_memory)?; - } - - if tail_bytes > 0 { - let (_guard, dst_page) = unsafe { - MmapGuard::map_mut_anywhere( - *grants_fd, - first_aligned_page + remaining_page_count * PAGE_SIZE, - PAGE_SIZE, - )? - }; - read_all(*image_file, None, &mut dst_page[..tail_bytes])?; + read_all(*image_file, None, &mut dst_memory[voff..voff + filesz])?; } // file_page_count..file_page_count + zero_page_count are already zero-initialized From 050edcdcc6fcd1405beb5afdc39ce97bdbbc2767 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 12 Apr 2025 20:13:39 +0200 Subject: [PATCH 2/4] Pass PROT_READ to mmap when the segment has PF_R This will allow the kernel to not add an implicit PROT_READ for every mmap in the future. --- redox-rt/src/proc.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs index b6e1047d13..65a141fdc8 100644 --- a/redox-rt/src/proc.rs +++ b/redox-rt/src/proc.rs @@ -8,12 +8,12 @@ use alloc::{boxed::Box, collections::BTreeMap, vec}; #[cfg(target_pointer_width = "32")] use goblin::elf32::{ header::Header, - program_header::program_header32::{ProgramHeader, PF_W, PF_X, PT_INTERP, PT_LOAD}, + program_header::program_header32::{ProgramHeader, PF_R, PF_W, PF_X, PT_INTERP, PT_LOAD}, }; #[cfg(target_pointer_width = "64")] use goblin::elf64::{ header::Header, - program_header::program_header64::{ProgramHeader, PF_W, PF_X, PT_INTERP, PT_LOAD}, + program_header::program_header64::{ProgramHeader, PF_R, PF_W, PF_X, PT_INTERP, PT_LOAD}, }; use syscall::{ @@ -114,7 +114,11 @@ where let ph_bytes = &phs[ph_idx * phentsize..(ph_idx + 1) * phentsize]; let segment: &ProgramHeader = plain::from_bytes(ph_bytes).map_err(|_| Error::new(EINVAL))?; - let mut flags = syscall::PROT_READ; + let mut flags = if segment.p_flags & PF_R == PF_R { + syscall::PROT_READ + } else { + syscall::PROT_NONE + }; // W ^ X. If it is executable, do not allow it to be writable, even if requested if segment.p_flags & PF_X == PF_X { From 17172720a749fc7795d3bc504636bec9825a2cea Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 12 Apr 2025 21:49:42 +0200 Subject: [PATCH 3/4] Always pass offset to read_all and rename to pread_all --- redox-rt/src/proc.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs index 65a141fdc8..d51c9f8010 100644 --- a/redox-rt/src/proc.rs +++ b/redox-rt/src/proc.rs @@ -78,7 +78,7 @@ where // entirely. let mut header_bytes = [0_u8; size_of::
()]; - read_all(*image_file, Some(0), &mut header_bytes)?; + pread_all(*image_file, 0, &mut header_bytes)?; let header = Header::from_bytes(&header_bytes); let grants_fd = { @@ -107,8 +107,7 @@ where |o| core::mem::take(&mut o.tree), ); - read_all(*image_file as usize, Some(header.e_phoff as u64), phs) - .map_err(|_| Error::new(EIO))?; + pread_all(*image_file as usize, header.e_phoff, phs).map_err(|_| Error::new(EIO))?; for ph_idx in 0..phnum { let ph_bytes = &phs[ph_idx * phentsize..(ph_idx + 1) * phentsize]; @@ -131,11 +130,7 @@ where // PT_INTERP must come before any PT_LOAD, so we don't have to iterate twice. PT_INTERP => { let mut interp = vec![0_u8; segment.p_filesz as usize]; - read_all( - *image_file as usize, - Some(segment.p_offset as u64), - &mut interp, - )?; + pread_all(*image_file as usize, segment.p_offset, &mut interp)?; return Ok(FexecResult::Interp { path: interp.into_boxed_slice(), @@ -176,9 +171,6 @@ where // TODO: Attempt to mmap with MAP_PRIVATE directly from the image file instead. if filesz > 0 { - syscall::lseek(*image_file, segment.p_offset as isize, SEEK_SET) - .map_err(|_| Error::new(EIO))?; - let (_guard, dst_memory) = unsafe { MmapGuard::map_mut_anywhere( *grants_fd, @@ -186,7 +178,11 @@ where (voff + filesz).next_multiple_of(PAGE_SIZE), // size )? }; - read_all(*image_file, None, &mut dst_memory[voff..voff + filesz])?; + pread_all( + *image_file, + segment.p_offset, + &mut dst_memory[voff..voff + filesz], + )?; } // file_page_count..file_page_count + zero_page_count are already zero-initialized @@ -530,10 +526,8 @@ pub fn munmap_transfer( ], ) } -fn read_all(fd: usize, offset: Option, buf: &mut [u8]) -> Result<()> { - if let Some(offset) = offset { - syscall::lseek(fd, offset as isize, SEEK_SET)?; - } +fn pread_all(fd: usize, offset: u64, buf: &mut [u8]) -> Result<()> { + syscall::lseek(fd, offset as isize, SEEK_SET)?; let mut total_bytes_read = 0; From ea4c41686c3754604b20647275ccd99438ead3f3 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 12 Apr 2025 22:15:05 +0200 Subject: [PATCH 4/4] Rustfmt --- src/header/signal/mod.rs | 10 ++++++++-- src/header/sys_uio/mod.rs | 14 ++++++++++++-- src/platform/redox/mod.rs | 7 ++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/header/signal/mod.rs b/src/header/signal/mod.rs index 8570ba54e8..77189fc98b 100644 --- a/src/header/signal/mod.rs +++ b/src/header/signal/mod.rs @@ -93,10 +93,16 @@ global_asm!(include_str!("sigsetjmp/aarch64/sigsetjmp.s")); global_asm!(include_str!("sigsetjmp/riscv64/sigsetjmp.s")); #[cfg(target_arch = "x86")] -global_asm!(include_str!("sigsetjmp/i386/sigsetjmp.s"), options(att_syntax)); +global_asm!( + include_str!("sigsetjmp/i386/sigsetjmp.s"), + options(att_syntax) +); #[cfg(target_arch = "x86_64")] -global_asm!(include_str!("sigsetjmp/x86_64/sigsetjmp.s"), options(att_syntax)); +global_asm!( + include_str!("sigsetjmp/x86_64/sigsetjmp.s"), + options(att_syntax) +); extern "C" { pub fn sigsetjmp(jb: *mut u64, savemask: i32) -> i32; diff --git a/src/header/sys_uio/mod.rs b/src/header/sys_uio/mod.rs index f54926554f..418302acbf 100644 --- a/src/header/sys_uio/mod.rs +++ b/src/header/sys_uio/mod.rs @@ -40,7 +40,12 @@ unsafe fn scatter(iovs: &[iovec], vec: Vec) { } #[no_mangle] -pub unsafe extern "C" fn preadv(fd: c_int, iov: *const iovec, iovcnt: c_int, offset: off_t) -> ssize_t { +pub unsafe extern "C" fn preadv( + fd: c_int, + iov: *const iovec, + iovcnt: c_int, + offset: off_t, +) -> ssize_t { if iovcnt < 0 || iovcnt > IOV_MAX { platform::ERRNO.set(errno::EINVAL); return -1; @@ -57,7 +62,12 @@ pub unsafe extern "C" fn preadv(fd: c_int, iov: *const iovec, iovcnt: c_int, off } #[no_mangle] -pub unsafe extern "C" fn pwritev(fd: c_int, iov: *const iovec, iovcnt: c_int, offset: off_t) -> ssize_t { +pub unsafe extern "C" fn pwritev( + fd: c_int, + iov: *const iovec, + iovcnt: c_int, + offset: off_t, +) -> ssize_t { if iovcnt < 0 || iovcnt > IOV_MAX { platform::ERRNO.set(errno::EINVAL); return -1; diff --git a/src/platform/redox/mod.rs b/src/platform/redox/mod.rs index 9018be61ac..f3cfa0b137 100644 --- a/src/platform/redox/mod.rs +++ b/src/platform/redox/mod.rs @@ -503,7 +503,12 @@ impl Pal for Sys { fn gettid() -> pid_t { // This is used by pthread mutexes for reentrant checks and must be nonzero // and unique for each thread in the same process (but not cross-process) - Self::current_os_tid().thread_fd.checked_add(1).unwrap().try_into().unwrap() + Self::current_os_tid() + .thread_fd + .checked_add(1) + .unwrap() + .try_into() + .unwrap() } unsafe fn gettimeofday(tp: *mut timeval, tzp: *mut timezone) -> Result<()> {