From 2eb287bf07a03d56eba0671f8be7bf6a82cfb5f9 Mon Sep 17 00:00:00 2001 From: Alex Zenla Date: Tue, 30 Jan 2024 18:02:32 -0800 Subject: [PATCH] xenclient: convert to using thiserror --- xen/xenclient/src/boot.rs | 20 ++----- xen/xenclient/src/elfloader.rs | 59 +++++---------------- xen/xenclient/src/error.rs | 95 +++++++++++++--------------------- xen/xenclient/src/lib.rs | 14 ++--- xen/xenclient/src/mem.rs | 20 ++++--- xen/xenclient/src/x86.rs | 20 +++---- 6 files changed, 78 insertions(+), 150 deletions(-) diff --git a/xen/xenclient/src/boot.rs b/xen/xenclient/src/boot.rs index f7860a4..a0ffb8a 100644 --- a/xen/xenclient/src/boot.rs +++ b/xen/xenclient/src/boot.rs @@ -182,7 +182,7 @@ impl BootSetup<'_> { let addr = self .call .mmap(0, 1 << XEN_PAGE_SHIFT) - .ok_or(Error::new("failed to mmap for resource"))?; + .ok_or(Error::MmapFailed)?; self.call.map_resource(self.domid, 1, 0, 0, 1, addr)?; let entries = unsafe { slice::from_raw_parts_mut(addr as *mut GrantEntry, 2) }; entries[0].flags = 1 << 0; @@ -194,7 +194,7 @@ impl BootSetup<'_> { unsafe { let result = munmap(addr as *mut c_void, 1 << XEN_PAGE_SHIFT); if result != 0 { - return Err(Error::new("failed to unmap resource")); + return Err(Error::UnmapFailed); } } Ok(()) @@ -294,15 +294,11 @@ impl BootSetup<'_> { fn alloc_padding_pages(&mut self, arch: &mut dyn ArchBootSetup, boundary: u64) -> Result<()> { if (boundary & (arch.page_size() - 1)) != 0 { - return Err(Error::new( - format!("segment boundary isn't page aligned: {:#x}", boundary).as_str(), - )); + return Err(Error::MemorySetupFailed); } if boundary < self.virt_alloc_end { - return Err(Error::new( - format!("segment boundary too low: {:#x})", boundary).as_str(), - )); + return Err(Error::MemorySetupFailed); } let pages = (boundary - self.virt_alloc_end) / arch.page_size(); self.chk_alloc_pages(arch, pages)?; @@ -314,13 +310,7 @@ impl BootSetup<'_> { || self.pfn_alloc_end > self.total_pages || pages > self.total_pages - self.pfn_alloc_end { - return Err(Error::new( - format!( - "segment too large: pages={} total_pages={} pfn_alloc_end={}", - pages, self.total_pages, self.pfn_alloc_end - ) - .as_str(), - )); + return Err(Error::MemorySetupFailed); } self.pfn_alloc_end += pages; diff --git a/xen/xenclient/src/elfloader.rs b/xen/xenclient/src/elfloader.rs index 900f52c..c9841d9 100644 --- a/xen/xenclient/src/elfloader.rs +++ b/xen/xenclient/src/elfloader.rs @@ -8,35 +8,16 @@ use crate::Error; use elf::abi::{PF_R, PF_W, PF_X, PT_LOAD, SHT_NOTE}; use elf::endian::AnyEndian; use elf::note::Note; -use elf::{ElfBytes, ParseError}; +use elf::ElfBytes; use flate2::bufread::GzDecoder; use log::debug; use memchr::memmem::find_iter; use slice_copy::copy; use std::collections::HashMap; -use std::ffi::{FromVecWithNulError, IntoStringError}; use std::io::{BufReader, Read}; use std::mem::size_of; use xz2::bufread::XzDecoder; -impl From for Error { - fn from(value: ParseError) -> Self { - Error::new(value.to_string().as_str()) - } -} - -impl From for Error { - fn from(value: FromVecWithNulError) -> Self { - Error::new(value.to_string().as_str()) - } -} - -impl From for Error { - fn from(value: IntoStringError) -> Self { - Error::new(value.to_string().as_str()) - } -} - pub struct ElfImageLoader { data: Vec, } @@ -146,9 +127,7 @@ impl ElfImageLoader { } } - Err(Error::new( - "Unable to parse kernel image: unknown compression type", - )) + Err(Error::ElfCompressionUnknown) } } @@ -159,9 +138,7 @@ struct ElfNoteValue { impl BootImageLoader for ElfImageLoader { fn parse(&self) -> Result { let elf = ElfBytes::::minimal_parse(self.data.as_slice())?; - let headers = elf.section_headers().ok_or(Error::new( - "Unable to parse kernel image: section headers not found.", - ))?; + let headers = elf.section_headers().ok_or(Error::ElfInvalidImage)?; let mut linux_notes: HashMap> = HashMap::new(); let mut xen_notes: HashMap = HashMap::new(); @@ -198,46 +175,42 @@ impl BootImageLoader for ElfImageLoader { } if linux_notes.is_empty() { - return Err(Error::new( - "Provided kernel does not appear to be a Linux kernel image.", - )); + return Err(Error::ElfInvalidImage); } if xen_notes.is_empty() { - return Err(Error::new("Provided kernel does not have Xen support.")); + return Err(Error::ElfXenSupportMissing); } let paddr_offset = xen_notes .get(&XEN_ELFNOTE_PADDR_OFFSET) - .ok_or(Error::new("Unable to find paddr_offset note in kernel."))? + .ok_or(Error::ElfInvalidImage)? .value; let virt_base = xen_notes .get(&XEN_ELFNOTE_VIRT_BASE) - .ok_or(Error::new("Unable to find virt_base note in kernel."))? + .ok_or(Error::ElfInvalidImage)? .value; let entry = xen_notes .get(&XEN_ELFNOTE_ENTRY) - .ok_or(Error::new("Unable to find entry note in kernel."))? + .ok_or(Error::ElfInvalidImage)? .value; let virt_hypercall = xen_notes .get(&XEN_ELFNOTE_HYPERCALL_PAGE) - .ok_or(Error::new("Unable to find hypercall_page note in kernel."))? + .ok_or(Error::ElfInvalidImage)? .value; let init_p2m = xen_notes .get(&XEN_ELFNOTE_INIT_P2M) - .ok_or(Error::new("Unable to find init_p2m note in kernel."))? + .ok_or(Error::ElfInvalidImage)? .value; let mod_start_pfn = xen_notes .get(&XEN_ELFNOTE_MOD_START_PFN) - .ok_or(Error::new("Unable to find mod_start_pfn note in kernel."))? + .ok_or(Error::ElfInvalidImage)? .value; let mut start: u64 = u64::MAX; let mut end: u64 = 0; - let segments = elf.segments().ok_or(Error::new( - "Unable to parse kernel image: segments not found.", - ))?; + let segments = elf.segments().ok_or(Error::ElfInvalidImage)?; for header in segments { if (header.p_type != PT_LOAD) || (header.p_flags & (PF_R | PF_W | PF_X)) == 0 { @@ -255,9 +228,7 @@ impl BootImageLoader for ElfImageLoader { } if paddr_offset != XEN_UNSET_ADDR && virt_base == XEN_UNSET_ADDR { - return Err(Error::new( - "Unable to load kernel image: paddr_offset set but virt_base is unset.", - )); + return Err(Error::ElfInvalidImage); } let virt_offset = virt_base - paddr_offset; @@ -280,9 +251,7 @@ impl BootImageLoader for ElfImageLoader { fn load(&self, image_info: &BootImageInfo, dst: &mut [u8]) -> Result<()> { let elf = ElfBytes::::minimal_parse(self.data.as_slice())?; - let segments = elf.segments().ok_or(Error::new( - "Unable to parse kernel image: segments not found.", - ))?; + let segments = elf.segments().ok_or(Error::ElfInvalidImage)?; debug!( "ElfImageLoader load dst={:#x} segments={}", diff --git a/xen/xenclient/src/error.rs b/xen/xenclient/src/error.rs index 5020bd7..20c432b 100644 --- a/xen/xenclient/src/error.rs +++ b/xen/xenclient/src/error.rs @@ -1,60 +1,39 @@ -use std::fmt::{Display, Formatter}; -use std::string::FromUtf8Error; -use xencall::XenCallError; +use std::io; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("io issue encountered")] + Io(#[from] io::Error), + #[error("xenstore issue encountered")] + XenStore(#[from] xenstore::error::Error), + #[error("xencall issue encountered")] + XenCall(#[from] xencall::XenCallError), + #[error("domain does not have a tty")] + TtyNotFound, + #[error("introducing the domain failed")] + IntroduceDomainFailed, + #[error("string conversion of a path failed")] + PathStringConversion, + #[error("parent of path not found")] + PathParentNotFound, + #[error("domain does not exist")] + DomainNonExistent, + #[error("elf parse failed")] + ElfParseFailed(#[from] elf::ParseError), + #[error("mmap failed")] + MmapFailed, + #[error("munmap failed")] + UnmapFailed, + #[error("memory setup failed")] + MemorySetupFailed, + #[error("populate physmap failed: wanted={0}, received={1}, input_extents={2}")] + PopulatePhysmapFailed(usize, usize, usize), + #[error("unknown elf compression method")] + ElfCompressionUnknown, + #[error("expected elf image format not found")] + ElfInvalidImage, + #[error("provided elf image does not contain xen support")] + ElfXenSupportMissing, +} pub type Result = std::result::Result; - -#[derive(Debug)] -pub struct Error { - message: String, -} - -impl Error { - pub fn new(msg: &str) -> Error { - Error { - message: msg.to_string(), - } - } -} - -impl Display for Error { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.message) - } -} - -impl std::error::Error for Error { - fn description(&self) -> &str { - &self.message - } -} - -impl From for Error { - fn from(value: std::io::Error) -> Self { - Error::new(value.to_string().as_str()) - } -} - -impl From for Error { - fn from(value: xenstore::error::Error) -> Self { - Error::new(value.to_string().as_str()) - } -} - -impl From for Error { - fn from(value: XenCallError) -> Self { - Error::new(value.to_string().as_str()) - } -} - -impl From for Error { - fn from(value: FromUtf8Error) -> Self { - Error::new(value.to_string().as_str()) - } -} - -impl From for Error { - fn from(value: xenevtchn::error::Error) -> Self { - Error::new(value.to_string().as_str()) - } -} diff --git a/xen/xenclient/src/lib.rs b/xen/xenclient/src/lib.rs index 79c9141..7b1f752 100644 --- a/xen/xenclient/src/lib.rs +++ b/xen/xenclient/src/lib.rs @@ -98,7 +98,7 @@ impl XenClient { let dom_path = self.store.get_domain_path(domid)?; let vm_path = self.store.read_string(&format!("{}/vm", dom_path))?; if vm_path.is_empty() { - return Err(Error::new("cannot destroy domain that doesn't exist")); + return Err(Error::DomainNonExistent); } let mut backend_paths: Vec = Vec::new(); @@ -161,12 +161,8 @@ impl XenClient { } for path in &backend_removals { let path = PathBuf::from(path); - let parent = path - .parent() - .ok_or(Error::new("unable to get parent of backend path"))?; - tx.rm(parent - .to_str() - .ok_or(Error::new("unable to convert parent to string"))?)?; + let parent = path.parent().ok_or(Error::PathParentNotFound)?; + tx.rm(parent.to_str().ok_or(Error::PathStringConversion)?)?; } tx.rm(&vm_path)?; tx.rm(&dom_path)?; @@ -336,7 +332,7 @@ impl XenClient { .store .introduce_domain(domid, xenstore_mfn, xenstore_evtchn)? { - return Err(Error::new("failed to introduce domain")); + return Err(Error::IntroduceDomainFailed); } self.console_device_add( &dom_path, @@ -570,7 +566,7 @@ impl XenClient { .read_string_optional(&console_tty_path)? .unwrap_or("".to_string()); if tty.is_empty() { - return Err(Error::new(&format!("domain {} does not have a tty", domid))); + return Err(Error::TtyNotFound); } let read = OpenOptions::new().read(true).write(false).open(&tty)?; let write = OpenOptions::new().read(false).write(true).open(&tty)?; diff --git a/xen/xenclient/src/mem.rs b/xen/xenclient/src/mem.rs index 355dcf0..589c366 100644 --- a/xen/xenclient/src/mem.rs +++ b/xen/xenclient/src/mem.rs @@ -53,7 +53,7 @@ impl PhysicalPages<'_> { } if pfn < page.pfn || (pfn + count) > page.pfn + page.count { - return Err(Error::new("request overlaps allocated block")); + return Err(Error::MemorySetupFailed); } } else { if pfn < page.pfn { @@ -69,9 +69,7 @@ impl PhysicalPages<'_> { } if count == 0 { - return Err(Error::new( - "allocation is only allowed when a size is given", - )); + return Err(Error::MemorySetupFailed); } self.pfn_alloc(pfn, count) @@ -96,11 +94,11 @@ impl PhysicalPages<'_> { let addr = self .call .mmap(0, actual_mmap_len) - .ok_or(Error::new("failed to mmap address"))?; + .ok_or(Error::MmapFailed)?; debug!("mapped {:#x} foreign bytes at {:#x}", actual_mmap_len, addr); let result = self.call.mmap_batch(self.domid, num as u64, addr, pfns)?; if result != 0 { - return Err(Error::new("mmap_batch call failed")); + return Err(Error::MmapFailed); } let page = PhysicalPage { pfn, @@ -126,11 +124,11 @@ impl PhysicalPages<'_> { let addr = self .call .mmap(0, actual_mmap_len) - .ok_or(Error::new("failed to mmap address"))?; + .ok_or(Error::MmapFailed)?; debug!("mapped {:#x} foreign bytes at {:#x}", actual_mmap_len, addr); let result = self.call.mmap_batch(self.domid, num as u64, addr, pfns)?; if result != 0 { - return Err(Error::new("mmap_batch call failed")); + return Err(Error::MmapFailed); } let page = PhysicalPage { pfn: u64::MAX, @@ -153,7 +151,7 @@ impl PhysicalPages<'_> { (page.count << X86_PAGE_SHIFT) as usize, ); if err != 0 { - return Err(Error::new("failed to munmap all pages")); + return Err(Error::UnmapFailed); } } } @@ -164,7 +162,7 @@ impl PhysicalPages<'_> { pub fn unmap(&mut self, pfn: u64) -> Result<()> { let page = self.pages.iter().enumerate().find(|(_, x)| x.pfn == pfn); if page.is_none() { - return Err(Error::new("unable to find page to unmap")); + return Err(Error::MemorySetupFailed); } let (i, page) = page.unwrap(); @@ -179,7 +177,7 @@ impl PhysicalPages<'_> { page.ptr ); if err != 0 { - return Err(Error::new("failed to munmap page")); + return Err(Error::UnmapFailed); } self.pages.remove(i); } diff --git a/xen/xenclient/src/x86.rs b/xen/xenclient/src/x86.rs index c164c2c..c89dd32 100644 --- a/xen/xenclient/src/x86.rs +++ b/xen/xenclient/src/x86.rs @@ -203,19 +203,19 @@ impl X86BootSetup { ) -> Result { debug!("counting pgtables from={} to={} pfn={}", from, to, pfn); if self.table.mappings_count == X86_PAGE_TABLE_MAX_MAPPINGS { - return Err(Error::new("too many mappings")); + return Err(Error::MemorySetupFailed); } let m = self.table.mappings_count; let pfn_end = pfn + ((to - from) >> X86_PAGE_SHIFT); if pfn_end >= setup.phys.p2m_size() { - return Err(Error::new("not enough memory for initial mapping")); + return Err(Error::MemorySetupFailed); } for idx in 0..self.table.mappings_count { if from < self.table.mappings[idx].area.to && to > self.table.mappings[idx].area.from { - return Err(Error::new("overlapping mappings")); + return Err(Error::MemorySetupFailed); } } let mut map = PageTableMapping::default(); @@ -494,7 +494,7 @@ impl ArchBootSetup for X86BootSetup { } if total != total_pages { - return Err(Error::new("page count mismatch while calculating pages")); + return Err(Error::MemorySetupFailed); } setup.total_pages = total; @@ -561,14 +561,10 @@ impl ArchBootSetup for X86BootSetup { .populate_physmap(setup.domid, allocsz, 0, 0, input_extent_starts)?; if result.len() != allocsz as usize { - return Err(Error::new( - format!( - "failed to populate physmap: wanted={} received={} input_extents={}", - allocsz, - result.len(), - input_extent_starts.len() - ) - .as_str(), + return Err(Error::PopulatePhysmapFailed( + allocsz as usize, + result.len(), + input_extent_starts.len(), )); }