diff --git a/kernel-rs/Cargo.lock b/kernel-rs/Cargo.lock index e74cfa5c2..33c897454 100644 --- a/kernel-rs/Cargo.lock +++ b/kernel-rs/Cargo.lock @@ -24,6 +24,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "byteorder" +version = "1.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" + [[package]] name = "cstr_core" version = "0.2.2" @@ -143,6 +149,7 @@ dependencies = [ "scopeguard", "spin", "static_assertions", + "zerocopy", ] [[package]] @@ -174,8 +181,41 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "synstructure" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b834f2d66f734cb897113e34aaff2f1ab4719ca946f9a7358dba8f8064148701" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "unicode-xid", +] + [[package]] name = "unicode-xid" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f7fe0bb3479651439c9112f72b6c505038574c9fbb575ed1bf3b797fa39dd564" + +[[package]] +name = "zerocopy" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6580539ad917b7c026220c4b3f2c08d52ce54d6ce0dc491e66002e35388fab46" +dependencies = [ + "byteorder", + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d498dbd1fd7beb83c86709ae1c33ca50942889473473d287d56ce4770a18edfb" +dependencies = [ + "proc-macro2", + "syn", + "synstructure", +] diff --git a/kernel-rs/Cargo.toml b/kernel-rs/Cargo.toml index e36020265..59bb65be4 100644 --- a/kernel-rs/Cargo.toml +++ b/kernel-rs/Cargo.toml @@ -30,6 +30,7 @@ array-macro = "2.0.0" static_assertions = "1.1.0" itertools = { version = "0.10.0", default-features = false } pin-project = "1" +zerocopy = { version = "0.3.0", default-features = false } # Compiler options for sysroot packages. # Cargo currently warns following packages are not dependencies. diff --git a/kernel-rs/src/exec.rs b/kernel-rs/src/exec.rs index 7f02738fc..ea098edb2 100644 --- a/kernel-rs/src/exec.rs +++ b/kernel-rs/src/exec.rs @@ -4,6 +4,7 @@ use core::{cmp, mem}; use bitflags::bitflags; use itertools::*; +use zerocopy::{AsBytes, FromBytes}; use crate::{ fs::Path, @@ -22,10 +23,13 @@ const ELF_MAGIC: u32 = 0x464c457f; const ELF_PROG_LOAD: u32 = 1; /// File header -#[derive(Default, Clone)] +#[derive(Default, Clone, AsBytes, FromBytes)] // It needs repr(C) because it's struct for in-disk representation // which should follow C(=machine) representation // https://github.com/kaist-cp/rv6/issues/52 +// repr(C) is also required for AsBytes & FromBytes. +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.AsBytes.html +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.FromBytes.html #[repr(C)] struct ElfHdr { /// must equal ELF_MAGIC @@ -48,6 +52,10 @@ struct ElfHdr { bitflags! { /// Flag bits for ProgHdr flags + #[derive(AsBytes, FromBytes)] + // repr(C) is also required for AsBytes & FromBytes. + // https://docs.rs/zerocopy/0.3.0/zerocopy/trait.AsBytes.html + // https://docs.rs/zerocopy/0.3.0/zerocopy/trait.FromBytes.html #[repr(C)] struct ProgFlags: u32 { const EXEC = 1; @@ -63,10 +71,13 @@ impl Default for ProgFlags { } /// Program section header -#[derive(Default, Clone)] +#[derive(Default, Clone, AsBytes, FromBytes)] // It needs repr(C) because it's struct for in-disk representation // which should follow C(=machine) representation // https://github.com/kaist-cp/rv6/issues/52 +// repr(C) is also required for AsBytes & FromBytes. +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.AsBytes.html +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.FromBytes.html #[repr(C)] struct ProgHdr { typ: u32, @@ -113,9 +124,7 @@ impl Kernel { // Check ELF header let mut elf: ElfHdr = Default::default(); - // SAFETY: ElfHdr can be safely transmuted to [u8; _], as it - // contains only integers, which do not have internal structures. - unsafe { ip.read_kernel(&mut elf, 0) }?; + ip.read_kernel(&mut elf, 0)?; if !elf.is_valid() { return Err(()); } @@ -128,9 +137,7 @@ impl Kernel { let off = elf.phoff + i * mem::size_of::(); let mut ph: ProgHdr = Default::default(); - // SAFETY: ProgHdr can be safely transmuted to [u8; _], as it - // contains only integers, which do not have internal structures. - unsafe { ip.read_kernel(&mut ph, off as _) }?; + ip.read_kernel(&mut ph, off as _)?; if ph.is_prog_load() { if ph.memsz < ph.filesz || ph.vaddr % PGSIZE != 0 { return Err(()); diff --git a/kernel-rs/src/fs/inode.rs b/kernel-rs/src/fs/inode.rs index 505a92acb..7588b84ee 100644 --- a/kernel-rs/src/fs/inode.rs +++ b/kernel-rs/src/fs/inode.rs @@ -76,6 +76,7 @@ use core::{ use array_macro::array; use static_assertions::const_assert; +use zerocopy::{AsBytes, FromBytes}; use super::{FileName, IPB, MAXFILE, NDIRECT, NINDIRECT}; use crate::{ @@ -187,7 +188,12 @@ pub struct InodeGuard<'a> { pub inode: &'a Inode, } -#[derive(Default)] +#[derive(Default, AsBytes, FromBytes)] +// It needs repr(C) for deriving zerocopy::FromBytes trait. +// DIRSIZ should match conditions for AsBytes & FromBytes. +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.AsBytes.html +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.FromBytes.html +#[repr(C)] pub struct Dirent { pub inum: u16, name: [u8; DIRSIZ], @@ -196,9 +202,7 @@ pub struct Dirent { impl Dirent { fn new(ip: &mut InodeGuard<'_>, off: u32) -> Result { let mut dirent = Dirent::default(); - // SAFETY: Dirent can be safely transmuted to [u8; _], as it - // contains only u16 and u8's, which do not have internal structures. - unsafe { ip.read_kernel(&mut dirent, off) }?; + ip.read_kernel(&mut dirent, off)?; Ok(dirent) } @@ -409,16 +413,8 @@ impl InodeGuard<'_> { /// Copy data into `dst` from the content of inode at offset `off`. /// Return Ok(()) on success, Err(()) on failure. - /// - /// # Safety - /// - /// `T` can be safely `transmute`d to `[u8; size_of::()]`. - pub unsafe fn read_kernel(&mut self, dst: &mut T, off: u32) -> Result<(), ()> { - let bytes = self.read_bytes_kernel( - // SAFETY: the safety assumption of this method. - unsafe { core::slice::from_raw_parts_mut(dst as *mut _ as _, mem::size_of::()) }, - off, - ); + pub fn read_kernel(&mut self, dst: &mut T, off: u32) -> Result<(), ()> { + let bytes = self.read_bytes_kernel(dst.as_bytes_mut(), off); if bytes == mem::size_of::() { Ok(()) } else { @@ -496,14 +492,13 @@ impl InodeGuard<'_> { /// Copy data from `src` into the inode at offset `off`. /// Return Ok(()) on success, Err(()) on failure. - pub fn write_kernel(&mut self, src: &T, off: u32, tx: &FsTransaction<'_>) -> Result<(), ()> { - let bytes = self.write_bytes_kernel( - // SAFETY: src is a valid reference to T and - // u8 does not have any internal structure. - unsafe { core::slice::from_raw_parts(src as *const _ as _, mem::size_of::()) }, - off, - tx, - )?; + pub fn write_kernel( + &mut self, + src: &T, + off: u32, + tx: &FsTransaction<'_>, + ) -> Result<(), ()> { + let bytes = self.write_bytes_kernel(src.as_bytes(), off, tx)?; if bytes == mem::size_of::() { Ok(()) } else { @@ -664,9 +659,8 @@ impl InodeGuard<'_> { pub fn is_dir_empty(&mut self) -> bool { let mut de: Dirent = Default::default(); for off in (2 * DIRENT_SIZE as u32..self.deref_inner().size).step_by(DIRENT_SIZE) { - // SAFETY: Dirent can be safely transmuted to [u8; _], as it - // contains only u16 and u8's, which do not have internal structures. - unsafe { self.read_kernel(&mut de, off) }.expect("is_dir_empty: read_kernel"); + self.read_kernel(&mut de, off) + .expect("is_dir_empty: read_kernel"); if de.inum != 0 { return false; } diff --git a/kernel-rs/src/stat.rs b/kernel-rs/src/stat.rs index 9b1f1a417..8b663e72b 100644 --- a/kernel-rs/src/stat.rs +++ b/kernel-rs/src/stat.rs @@ -1,5 +1,9 @@ -#[derive(Copy, Clone)] -#[repr(C)] +use zerocopy::{AsBytes, FromBytes}; + +#[derive(Copy, Clone, AsBytes, FromBytes)] +// repr(packed) is required for AsBytes. +// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.AsBytes.html +#[repr(packed)] pub struct Stat { /// File system's disk device pub dev: i32, diff --git a/kernel-rs/src/syscall.rs b/kernel-rs/src/syscall.rs index ddd40309d..856ca3f5f 100644 --- a/kernel-rs/src/syscall.rs +++ b/kernel-rs/src/syscall.rs @@ -17,8 +17,7 @@ pub fn fetchaddr(addr: UVAddr, proc: &mut CurrentProc<'_>) -> Result if addr.into_usize() >= proc.memory().size() || addr.into_usize() + sz > proc.memory().size() { return Err(()); } - // SAFETY: usize does not have any internal structure. - unsafe { proc.memory_mut().copy_in(&mut ip, addr) }?; + proc.memory_mut().copy_in(&mut ip, addr)?; Ok(ip) } diff --git a/kernel-rs/src/vm.rs b/kernel-rs/src/vm.rs index e986eb3d1..a717eb2e1 100644 --- a/kernel-rs/src/vm.rs +++ b/kernel-rs/src/vm.rs @@ -1,5 +1,7 @@ use core::{cmp, marker::PhantomData, mem, ops::Add, slice}; +use zerocopy::{AsBytes, FromBytes}; + use crate::{ fs::InodeGuard, kernel::kernel_builder, @@ -534,13 +536,8 @@ impl UserMemory { /// Copy from kernel to user. /// Copy from src to virtual address dstva in a given page table. /// Return Ok(()) on success, Err(()) on error. - pub fn copy_out(&mut self, dstva: UVAddr, src: &T) -> Result<(), ()> { - self.copy_out_bytes( - dstva, - // SAFETY: src is a valid reference to T and - // u8 does not have any internal structure. - unsafe { core::slice::from_raw_parts_mut(src as *const _ as _, mem::size_of::()) }, - ) + pub fn copy_out(&mut self, dstva: UVAddr, src: &T) -> Result<(), ()> { + self.copy_out_bytes(dstva, src.as_bytes()) } /// Copy from user to kernel. @@ -566,15 +563,12 @@ impl UserMemory { /// Copy from user to kernel. /// Copy to dst from virtual address srcva in a given page table. /// Return Ok(()) on success, Err(()) on error. - /// - /// # Safety - /// - /// `T` can be safely `transmute`d to `[u8; size_of::()]`. - pub unsafe fn copy_in(&mut self, dst: &mut T, srcva: UVAddr) -> Result<(), ()> { - self.copy_in_bytes( - unsafe { core::slice::from_raw_parts_mut(dst as *mut _ as _, mem::size_of::()) }, - srcva, - ) + pub fn copy_in( + &mut self, + dst: &mut T, + srcva: UVAddr, + ) -> Result<(), ()> { + self.copy_in_bytes(dst.as_bytes_mut(), srcva) } /// Copy a null-terminated string from user to kernel.