diff --git a/litebox_common_linux/src/mm.rs b/litebox_common_linux/src/mm.rs index 75346a172..0c66c4016 100644 --- a/litebox_common_linux/src/mm.rs +++ b/litebox_common_linux/src/mm.rs @@ -71,7 +71,16 @@ pub fn do_mmap< ProtFlags::PROT_NONE => unsafe { pm.create_inaccessible_pages(suggested_addr, length, flags, op) }, - _ => todo!("Unsupported prot flags {:?}", prot), + _ => { + #[cfg(debug_assertions)] + todo!("Unsupported prot flags {:?}", prot); + // TODO: create inaccessible pages for now. Creating mapping + // for both executable and writable might be needed for JIT. + #[cfg(not(debug_assertions))] + unsafe { + pm.create_inaccessible_pages(suggested_addr, length, flags, op) + } + } } } @@ -134,7 +143,12 @@ pub fn sys_mprotect< ProtFlags::PROT_READ => unsafe { pm.make_pages_readable(addr, len) }, ProtFlags::PROT_NONE => unsafe { pm.make_pages_inaccessible(addr, len) }, ProtFlags::PROT_READ_WRITE_EXEC => unsafe { pm.make_pages_rwx(addr, len) }, - _ => todo!("Unsupported prot flags {:?}", prot), + _ => { + #[cfg(debug_assertions)] + todo!("Unsupported prot flags {:?}", prot); + #[cfg(not(debug_assertions))] + return Err(Errno::EINVAL); + } } .map_err(Errno::from) } @@ -185,7 +199,10 @@ pub fn sys_mremap< } if flags.intersects(MRemapFlags::MREMAP_FIXED | MRemapFlags::MREMAP_DONTUNMAP) { + #[cfg(debug_assertions)] todo!("Unsupported flags {:?}", flags); + #[cfg(not(debug_assertions))] + return Err(Errno::EINVAL); } unsafe { diff --git a/litebox_platform_lvbs/src/arch/x86/mm/paging.rs b/litebox_platform_lvbs/src/arch/x86/mm/paging.rs index 4495c1e7a..82edf6824 100644 --- a/litebox_platform_lvbs/src/arch/x86/mm/paging.rs +++ b/litebox_platform_lvbs/src/arch/x86/mm/paging.rs @@ -317,38 +317,123 @@ impl X64PageTable<'_, M, ALIGN> { frame: _, offset: _, flags, - } => match inner.unmap(start) { - Ok((frame, _)) => { - match unsafe { inner.map_to(new_start, frame, flags, &mut allocator) } { - Ok(_) => {} - Err(e) => match e { - MapToError::PageAlreadyMapped(_) => { - return Err(page_mgmt::RemapError::AlreadyAllocated); + } => { + // Pre-check the destination so we never destroy the old PTE for a remap + // that can't complete. `translate(new_start)` reports `Mapped` for both + // present leaves and parent huge-page coverage, ruling out both + // `MapToError::PageAlreadyMapped` and `MapToError::ParentEntryHugePage` + // from the subsequent `map_to`. + match inner.translate(new_start.start_address()) { + TranslateResult::Mapped { .. } => { + return Err(page_mgmt::RemapError::AlreadyAllocated); + } + TranslateResult::InvalidFrameAddress(pa) => { + #[cfg(debug_assertions)] + todo!("Invalid frame address at remap destination: {:#x}", pa); + #[cfg(not(debug_assertions))] + { + crate::serial_println!( + "Invalid frame address at remap destination: {:#x}", + pa + ); + return Err(page_mgmt::RemapError::Unaligned); + } + } + TranslateResult::NotMapped => {} + } + match inner.unmap(start) { + Ok((frame, _)) => { + match unsafe { inner.map_to(new_start, frame, flags, &mut allocator) } { + Ok(_) => {} + Err(MapToError::FrameAllocationFailed) => { + // Best-effort: restore the page we just unmapped before + // bailing out. Earlier iterations of the loop have already + // migrated their pages and are NOT unwound here, so the + // caller may still observe a partial move on this error. + // `unmap` leaves the parent tables for `start` in place, so + // restoring the old mapping does not require allocation. + if let Err(rollback_err) = + unsafe { inner.map_to(start, frame, flags, &mut allocator) } + { + crate::serial_println!( + "BUG: remap rollback failed: {:?}", + rollback_err + ); + } + return Err(page_mgmt::RemapError::OutOfMemory); } - MapToError::ParentEntryHugePage => { - todo!("return Err(page_mgmt::RemapError::RemapToHugePage);") + // Ruled out by the pre-check above; if the destination + // state drifts from what `translate` reported, fall back + // to a structured error rather than panicking the kernel. + Err(MapToError::PageAlreadyMapped(_)) => { + debug_assert!( + false, + "BUG: map_to reported PageAlreadyMapped after pre-check at {:#x}", + new_start.start_address() + ); + crate::serial_println!( + "BUG: map_to reported PageAlreadyMapped after pre-check at {:#x}", + new_start.start_address() + ); + return Err(page_mgmt::RemapError::AlreadyAllocated); } - MapToError::FrameAllocationFailed => { - return Err(page_mgmt::RemapError::OutOfMemory); + Err(MapToError::ParentEntryHugePage) => { + debug_assert!( + false, + "BUG: map_to reported ParentEntryHugePage after pre-check at {:#x}", + new_start.start_address() + ); + crate::serial_println!( + "BUG: map_to reported ParentEntryHugePage after pre-check at {:#x}", + new_start.start_address() + ); + return Err(page_mgmt::RemapError::AlreadyAllocated); } - }, + } + } + Err(X64UnmapError::PageNotMapped) => { + debug_assert!( + false, + "BUG: unmap reported PageNotMapped after translate said Mapped at {:#x}", + start.start_address() + ); + crate::serial_println!( + "BUG: unmap reported PageNotMapped after translate said Mapped at {:#x}", + start.start_address() + ); + return Err(page_mgmt::RemapError::Unaligned); + } + Err(X64UnmapError::ParentEntryHugePage) => { + #[cfg(debug_assertions)] + todo!("return Err(page_mgmt::RemapError::RemapToHugePage);"); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("BUG: attempt to unmap a huge page"); + return Err(page_mgmt::RemapError::Unaligned); + } + } + Err(X64UnmapError::InvalidFrameAddress(pa)) => { + // TODO: `panic!()` -> `todo!()` because user-driven interrupts or exceptions must not halt the kernel. + // We should handle this exception carefully (i.e., clean up the context and data structures belonging to an erroneous process). + #[cfg(debug_assertions)] + todo!("Invalid frame address: {:#x}", pa); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Invalid frame address: {:#x}", pa); + return Err(page_mgmt::RemapError::Unaligned); + } } } - Err(X64UnmapError::PageNotMapped) => { - unreachable!() - } - Err(X64UnmapError::ParentEntryHugePage) => { - todo!("return Err(page_mgmt::RemapError::RemapToHugePage);") - } - Err(X64UnmapError::InvalidFrameAddress(pa)) => { - // TODO: `panic!()` -> `todo!()` because user-driven interrupts or exceptions must not halt the kernel. - // We should handle this exception carefully (i.e., clean up the context and data structures belonging to an errorneous process). - todo!("Invalid frame address: {:#x}", pa); - } - }, + } TranslateResult::NotMapped => {} TranslateResult::InvalidFrameAddress(pa) => { + #[cfg(debug_assertions)] todo!("Invalid frame address: {:#x}", pa); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Invalid frame address: {:#x}", pa); + return Err(page_mgmt::RemapError::Unaligned); + } } } start += 1; @@ -386,14 +471,11 @@ impl X64PageTable<'_, M, ALIGN> { offset: _, flags, } => { - // If it is changed to writable, we leave it to page fault handler (COW) - let change_to_write = new_flags.contains(PageTableFlags::WRITABLE) - && !flags.contains(PageTableFlags::WRITABLE); - let new_flags = if change_to_write { - new_flags - PageTableFlags::WRITABLE - } else { - new_flags - }; + // COW lazy-enable was unimplemented, so granting WRITABLE via a later + // fault would land in the unimplemented COW path and kill the task. + // Install the writable PTE directly until COW (and shared frames) land. + // FIXME: when COW is implemented, restore the lazy-enable masking that + // was removed here so a R->RW mprotect defers WRITABLE to the fault path. if flags != new_flags { match unsafe { inner.update_flags(page, (flags & !Self::MPROTECT_PTE_MASK) | new_flags) @@ -402,7 +484,15 @@ impl X64PageTable<'_, M, ALIGN> { Err(e) => match e { FlagUpdateError::PageNotMapped => unreachable!(), FlagUpdateError::ParentEntryHugePage => { - todo!("return Err(ProtectError::ProtectHugePage);") + #[cfg(debug_assertions)] + todo!("BUG: attempt to protect a huge page"); + #[cfg(not(debug_assertions))] + { + crate::serial_println!( + "BUG: attempt to protect a huge page" + ); + return Err(page_mgmt::PermissionUpdateError::Unaligned); + } } }, } @@ -410,7 +500,13 @@ impl X64PageTable<'_, M, ALIGN> { } TranslateResult::NotMapped => {} TranslateResult::InvalidFrameAddress(pa) => { + #[cfg(debug_assertions)] todo!("Invalid frame address: {:#x}", pa); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Invalid frame address: {:#x}", pa); + return Err(page_mgmt::PermissionUpdateError::Unaligned); + } } } } @@ -480,16 +576,25 @@ impl X64PageTable<'_, M, ALIGN> { offset: _, flags: _, } => { - assert!( - target_frame.start_address() == frame.start_address(), - "{page:?} is already mapped to {frame:?} instead of {target_frame:?}" - ); - + if target_frame.start_address() != frame.start_address() { + crate::serial_println!( + "BUG: {page:?} already mapped to {frame:?} instead of {target_frame:?}" + ); + return Err(MapToError::PageAlreadyMapped( + PhysFrame::::containing_address(frame.start_address()), + )); + } continue; } TranslateResult::NotMapped => {} TranslateResult::InvalidFrameAddress(pa) => { + #[cfg(debug_assertions)] todo!("Invalid frame address: {:#x}", pa); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Invalid frame address: {:#x}", pa); + return Err(MapToError::FrameAllocationFailed); + } } } @@ -781,7 +886,13 @@ impl PageTableImpl for X64PageTabl return Ok(()); } else { // Copy-on-Write + #[cfg(debug_assertions)] todo!("COW"); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("BUG: Copy-on-Write not implemented"); + return Err(PageFaultError::AllocationFailed); + } } } @@ -790,7 +901,16 @@ impl PageTableImpl for X64PageTabl return Ok(()); } + #[cfg(debug_assertions)] todo!("Page fault on present page: {:#x}", page.start_address()); + #[cfg(not(debug_assertions))] + { + crate::serial_println!( + "Page fault on present page: {:#x}", + page.start_address() + ); + return Err(PageFaultError::AccessError("Page fault on present page")); + } } TranslateResult::NotMapped => { let mut allocator = PageTableAllocator::::new(); @@ -826,7 +946,13 @@ impl PageTableImpl for X64PageTabl } } TranslateResult::InvalidFrameAddress(pa) => { + #[cfg(debug_assertions)] todo!("Invalid frame address: {:#x}", pa); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Invalid frame address: {:#x}", pa); + return Err(PageFaultError::AccessError("Invalid frame address")); + } } } Ok(()) diff --git a/litebox_platform_lvbs/src/lib.rs b/litebox_platform_lvbs/src/lib.rs index 48bc1c97a..e41f43ea0 100644 --- a/litebox_platform_lvbs/src/lib.rs +++ b/litebox_platform_lvbs/src/lib.rs @@ -1041,38 +1041,24 @@ impl RawMutex { val: u32, timeout: Option, ) -> Result { - loop { - // No need to wait if the value already changed. - if self - .underlying_atomic() - .load(core::sync::atomic::Ordering::Relaxed) - != val - { - return Err(ImmediatelyWokenUp); - } - - let ret = Host::block_or_maybe_timeout(&self.inner, val, timeout); + // No need to wait if the value already changed. + if self + .underlying_atomic() + .load(core::sync::atomic::Ordering::Relaxed) + != val + { + return Err(ImmediatelyWokenUp); + } - match ret { - Ok(()) => { - return Ok(UnblockedOrTimedOut::Unblocked); - } - Err(Errno::EAGAIN) => { - // If the futex value does not match val, then the call fails - // immediately with the error EAGAIN. - return Err(ImmediatelyWokenUp); - } - Err(Errno::EINTR) => { - // return Err(ImmediatelyWokenUp); - todo!("EINTR"); - } - Err(Errno::ETIMEDOUT) => { - return Ok(UnblockedOrTimedOut::TimedOut); - } - Err(e) => { - panic!("Error: {e:?}"); - } - } + #[allow(clippy::match_same_arms)] + match Host::block_or_maybe_timeout(&self.inner, val, timeout) { + Ok(()) => Ok(UnblockedOrTimedOut::Unblocked), + // If the futex value does not match val, then the call fails + // immediately with the error EAGAIN. + Err(Errno::EAGAIN) => Err(ImmediatelyWokenUp), + Err(Errno::EINTR) => Ok(UnblockedOrTimedOut::Unblocked), + Err(Errno::ETIMEDOUT) => Ok(UnblockedOrTimedOut::TimedOut), + Err(e) => panic!("Error: {e:?}"), } } } diff --git a/litebox_platform_lvbs/src/mshv/mem_integrity.rs b/litebox_platform_lvbs/src/mshv/mem_integrity.rs index 57eb5c572..f4ac3aef1 100644 --- a/litebox_platform_lvbs/src/mshv/mem_integrity.rs +++ b/litebox_platform_lvbs/src/mshv/mem_integrity.rs @@ -212,7 +212,13 @@ fn identify_direct_relocations( R_X86_64_64 => 8, R_X86_64_32 | R_X86_64_32S | R_X86_64_PLT32 | R_X86_64_PC32 => 4, _ => { + #[cfg(debug_assertions)] todo!("Unsupported relocation type {:?}", rela.r_type); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Unsupported relocation type {:?}", rela.r_type); + return Err(KernelElfError::UnsupportedRelocation); + } } }; let start = r_offset; @@ -300,7 +306,13 @@ fn identify_indirect_relocations( R_X86_64_64 => 8, R_X86_64_32 | R_X86_64_32S | R_X86_64_PLT32 | R_X86_64_PC32 => 4, _ => { + #[cfg(debug_assertions)] todo!("Unsupported relocation type {:?}", rela.r_type); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Unsupported relocation type {:?}", rela.r_type); + return Err(KernelElfError::UnsupportedRelocation); + } } }; @@ -386,12 +398,23 @@ pub fn verify_kernel_module_signature( let (signature, digest_alg, signature_alg) = decode_signature(signature_der)?; // We only support RSA with SHA-256 or SHA-512 for now as most Linux distributions use this combination. + #[allow(clippy::manual_assert)] if (digest_alg != ID_SHA_256 && digest_alg != ID_SHA_512) || (signature_alg != RSA_ENCRYPTION) { + #[cfg(debug_assertions)] todo!( "Unsupported digest or signature algorithm: {:?}, {:?}", digest_alg, signature_alg ); + #[cfg(not(debug_assertions))] + { + crate::serial_println!( + "Unsupported digest or signature algorithm: {:?}, {:?}", + digest_alg, + signature_alg + ); + return Err(VerificationError::Unsupported); + } } for cert in certs { let key_info = &cert.tbs_certificate.subject_public_key_info; @@ -530,8 +553,15 @@ pub fn verify_kernel_pe_signature( .to_der() .map_err(|_| VerificationError::InvalidSignature)?; let digest_algorithm_oid = authenticode_signature.signer_info().digest_alg.oid; + #[allow(clippy::manual_assert)] if digest_algorithm_oid != ID_SHA_256 && digest_algorithm_oid != ID_SHA_512 { + #[cfg(debug_assertions)] todo!("Unsupported digest algorithm: {:?}", digest_algorithm_oid); + #[cfg(not(debug_assertions))] + { + crate::serial_println!("Unsupported digest algorithm: {:?}", digest_algorithm_oid); + return Err(VerificationError::Unsupported); + } } let mut signature_verified = false; @@ -732,6 +762,9 @@ pub enum KernelElfError { ElfParseFailed, #[error("required section not found")] SectionNotFound, + #[cfg_attr(debug_assertions, allow(dead_code))] + #[error("unsupported relocation type")] + UnsupportedRelocation, } /// Errors for module signature verification. diff --git a/litebox_shim_optee/src/lib.rs b/litebox_shim_optee/src/lib.rs index 4aaecdeb7..a91fe4a6e 100644 --- a/litebox_shim_optee/src/lib.rs +++ b/litebox_shim_optee/src/lib.rs @@ -96,7 +96,10 @@ impl litebox::shim::EnterShim for OpteeShimEntrypoints { } fn interrupt(&self, _ctx: &mut Self::ExecutionContext) -> ContinueOperation { - todo!("Handle interrupt in OP-TEE shim"); + #[cfg(debug_assertions)] + todo!("OP-TEE shim doesn't support interrupt"); + #[cfg(not(debug_assertions))] + ContinueOperation::Terminate } } diff --git a/litebox_shim_optee/src/msg_handler.rs b/litebox_shim_optee/src/msg_handler.rs index e95a458f3..b1c125101 100644 --- a/litebox_shim_optee/src/msg_handler.rs +++ b/litebox_shim_optee/src/msg_handler.rs @@ -356,7 +356,13 @@ pub fn handle_optee_msg_args(msg_args: &OpteeMsgArgs) -> Result<(), OpteeSmcRetu | OpteeMessageCommand::InvokeCommand | OpteeMessageCommand::CloseSession => return Err(OpteeSmcReturnCode::Ok), _ => { + #[cfg(debug_assertions)] todo!("Unimplemented OpteeMessageCommand: {:?}", msg_args.cmd); + #[cfg(not(debug_assertions))] + { + litebox_util_log::debug!(cmd:? = msg_args.cmd; "Unimplemented OpteeMessageCommand"); + return Err(OpteeSmcReturnCode::EBadCmd); + } } } Ok(())