Skip to content

Commit

Permalink
Use zerocopy traits (closes #426)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeehoonkang committed Apr 30, 2021
1 parent 43d8af6 commit 80a3e86
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 31 deletions.
40 changes: 40 additions & 0 deletions kernel-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions kernel-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pin-project = "1.0.7"
scopeguard = { version = "1.1.0", default-features = false }
spin = "0.9.0"
static_assertions = "1.1.0"
zerocopy = "0.5.0"

# Compiler options for sysroot packages.
# Cargo currently warns following packages are not dependencies.
Expand Down
12 changes: 6 additions & 6 deletions kernel-rs/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use core::{cmp, mem};

use bitflags::bitflags;
use itertools::*;
use zerocopy::FromBytes;

use crate::{
arch::addr::{pgroundup, PAddr, PGSIZE},
Expand All @@ -26,6 +27,7 @@ const ELF_PROG_LOAD: u32 = 1;
// which should follow C(=machine) representation
// https://github.com/kaist-cp/rv6/issues/52
#[repr(C)]
#[derive(FromBytes)]
struct ElfHdr {
/// must equal ELF_MAGIC
magic: u32,
Expand All @@ -48,6 +50,7 @@ struct ElfHdr {
bitflags! {
/// Flag bits for ProgHdr flags
#[repr(C)]
#[derive(FromBytes)]
struct ProgFlags: u32 {
const EXEC = 1;
const WRITE = 2;
Expand All @@ -67,6 +70,7 @@ impl Default for ProgFlags {
// which should follow C(=machine) representation
// https://github.com/kaist-cp/rv6/issues/52
#[repr(C)]
#[derive(FromBytes)]
struct ProgHdr {
typ: u32,
flags: ProgFlags,
Expand Down Expand Up @@ -106,9 +110,7 @@ impl KernelCtx<'_, '_> {

// 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, self) }?;
ip.read_kernel(&mut elf, 0, self)?;
if !elf.is_valid() {
return Err(());
}
Expand All @@ -123,9 +125,7 @@ impl KernelCtx<'_, '_> {
let off = elf.phoff + i * mem::size_of::<ProgHdr>();

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 _, self) }?;
ip.read_kernel(&mut ph, off as _, self)?;
if ph.is_prog_load() {
if ph.memsz < ph.filesz || ph.vaddr % PGSIZE != 0 {
return Err(());
Expand Down
29 changes: 13 additions & 16 deletions kernel-rs/src/fs/inode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ use core::{
};

use static_assertions::const_assert;
use zerocopy::{AsBytes, FromBytes};

use super::{FileName, Stat, IPB, MAXFILE, NDIRECT, NINDIRECT};
use crate::{
Expand Down Expand Up @@ -183,7 +184,8 @@ pub struct InodeGuard<'a> {
pub inode: &'a Inode,
}

#[derive(Default)]
#[repr(C)]
#[derive(Default, AsBytes, FromBytes)]
pub struct Dirent {
pub inum: u16,
name: [u8; DIRSIZ],
Expand All @@ -192,9 +194,7 @@ pub struct Dirent {
impl Dirent {
fn new(ip: &mut InodeGuard<'_>, off: u32, ctx: &KernelCtx<'_, '_>) -> Result<Dirent, ()> {
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, ctx) }?;
ip.read_kernel(&mut dirent, off, ctx)?;
Ok(dirent)
}

Expand Down Expand Up @@ -409,18 +409,15 @@ 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::<T>()]`.
pub unsafe fn read_kernel<T>(
pub fn read_kernel<T: FromBytes>(
&mut self,
dst: &mut T,
off: u32,
ctx: &KernelCtx<'_, '_>,
) -> Result<(), ()> {
let bytes = self.read_bytes_kernel(
// SAFETY: the safety assumption of this method.
// SAFETY: `T` implements `FromBytes` and thus we can write to `dst` as if it's an `u8`
// buffer.
unsafe { core::slice::from_raw_parts_mut(dst as *mut _ as _, mem::size_of::<T>()) },
off,
ctx,
Expand Down Expand Up @@ -526,16 +523,16 @@ impl InodeGuard<'_> {

/// Copy data from `src` into the inode at offset `off`.
/// Return Ok(()) on success, Err(()) on failure.
pub fn write_kernel<T>(
pub fn write_kernel<T: AsBytes>(
&mut self,
src: &T,
off: u32,
tx: &FsTransaction<'_>,
ctx: &KernelCtx<'_, '_>,
) -> Result<(), ()> {
let bytes = self.write_bytes_kernel(
// SAFETY: src is a valid reference to T and
// u8 does not have any internal structure.
// SAFETY: `T` implements `AsBytes` and thus we can read `src` as if it's an `u8`
// buffer.
unsafe { core::slice::from_raw_parts(src as *const _ as _, mem::size_of::<T>()) },
off,
tx,
Expand Down Expand Up @@ -722,9 +719,8 @@ impl InodeGuard<'_> {
pub fn is_dir_empty(&mut self, ctx: &KernelCtx<'_, '_>) -> 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, ctx) }.expect("is_dir_empty: read_kernel");
self.read_kernel(&mut de, off, ctx)
.expect("is_dir_empty: read_kernel");
if de.inum != 0 {
return false;
}
Expand Down Expand Up @@ -874,6 +870,7 @@ impl Inode {
InodeType::Device { .. } => 3,
},
nlink: inner.nlink,
_padding: 0,
size: inner.size as usize,
}
}
Expand Down
7 changes: 6 additions & 1 deletion kernel-rs/src/fs/stat.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#[derive(Copy, Clone)]
use zerocopy::AsBytes;

#[derive(Copy, Clone, AsBytes)]
#[repr(C)]
pub struct Stat {
/// File system's disk device
Expand All @@ -13,6 +15,9 @@ pub struct Stat {
/// Number of links to file
pub nlink: i16,

/// Padding for safetly serializing the struct
pub _padding: u32,

/// Size of file in bytes
pub size: usize,
}
15 changes: 7 additions & 8 deletions kernel-rs/src/vm.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use core::{cmp, marker::PhantomData, mem, slice};

use bitflags::bitflags;
use zerocopy::{AsBytes, FromBytes};

use crate::{
arch::addr::{
Expand Down Expand Up @@ -540,11 +541,11 @@ 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<T>(&mut self, dstva: UVAddr, src: &T) -> Result<(), ()> {
pub fn copy_out<T: AsBytes>(&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.
// SAFETY: `T` implements `AsBytes` and thus we can read `dst` as if it's an `u8`
// buffer.
unsafe { core::slice::from_raw_parts_mut(src as *const _ as _, mem::size_of::<T>()) },
)
}
Expand Down Expand Up @@ -572,12 +573,10 @@ 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::<T>()]`.
pub unsafe fn copy_in<T>(&mut self, dst: &mut T, srcva: UVAddr) -> Result<(), ()> {
pub unsafe fn copy_in<T: FromBytes>(&mut self, dst: &mut T, srcva: UVAddr) -> Result<(), ()> {
self.copy_in_bytes(
// SAFETY: `T` implements `FromBytes` and thus we can write to `dst` as if it's an `u8`
// buffer.
unsafe { core::slice::from_raw_parts_mut(dst as *mut _ as _, mem::size_of::<T>()) },
srcva,
)
Expand Down

0 comments on commit 80a3e86

Please sign in to comment.