From ebc2b6253ab6df9b17329bf04d4084e7b1b69a01 Mon Sep 17 00:00:00 2001 From: belohnung Date: Mon, 10 Nov 2025 21:21:12 +0000 Subject: [PATCH] fix: replace panics with proper error handling This commit addresses several potential panic scenarios by introducing proper error handling. --- aya-obj/src/relocation.rs | 31 +++++++++++++---- aya/src/maps/info.rs | 8 +++-- aya/src/maps/mod.rs | 23 +++++++++--- aya/src/programs/info.rs | 8 +++-- aya/src/programs/links.rs | 20 +++++++++-- aya/src/programs/mod.rs | 54 +++++++++++++++++++++-------- aya/src/programs/raw_trace_point.rs | 5 ++- aya/src/programs/tc.rs | 3 +- aya/src/programs/xdp.rs | 7 ++-- xtask/public-api/aya-obj.txt | 3 ++ xtask/public-api/aya.txt | 14 ++++++++ 11 files changed, 140 insertions(+), 36 deletions(-) diff --git a/aya-obj/src/relocation.rs b/aya-obj/src/relocation.rs index d035339b..18533d94 100644 --- a/aya-obj/src/relocation.rs +++ b/aya-obj/src/relocation.rs @@ -85,6 +85,17 @@ pub enum RelocationError { /// The relocation number relocation_number: usize, }, + + /// Unsupported relocation + #[error( + "unsupported relocation target: symbol kind `{symbol_kind:?}` at symbol address #{relocation_number}" + )] + UnsupportedRelocationTarget { + /// The symbol kind + symbol_kind: SymbolKind, + /// The relocation number + relocation_number: u64, + }, } #[derive(Debug, Copy, Clone)] @@ -359,16 +370,19 @@ impl<'a> FunctionLinker<'a> { let rel = relocations .and_then(|relocations| { + let relocation_number = + (fun.section_offset + (ins_index - start_ins) * INS_SIZE) as u64; relocations - .get(&((fun.section_offset + (ins_index - start_ins) * INS_SIZE) as u64)) + .get(&relocation_number) + .map(|rel| (relocation_number, rel)) }) - .and_then(|rel| { + .and_then(|(rel_num, rel): (u64, &Relocation)| { // get the symbol for the relocation self.symbol_table .get(&rel.symbol_index) - .map(|sym| (rel, sym)) + .map(|sym| (rel_num, rel, sym)) }) - .filter(|(_rel, sym)| { + .filter(|(_rel_num, _rel, sym)| { // only consider text relocations, data relocations are // relocated in relocate_maps() sym.kind == SymbolKind::Text @@ -383,7 +397,7 @@ impl<'a> FunctionLinker<'a> { continue; } - let (callee_section_index, callee_address) = if let Some((rel, sym)) = rel { + let (callee_section_index, callee_address) = if let Some((rel_num, rel, sym)) = rel { let address = match sym.kind { SymbolKind::Text => sym.address, // R_BPF_64_32 this is a call @@ -392,7 +406,12 @@ impl<'a> FunctionLinker<'a> { } // R_BPF_64_64 this is a ld_imm64 text relocation SymbolKind::Section if rel.size == 64 => sym.address + ins.imm as u64, - _ => todo!(), // FIXME: return an error here, + symbol_kind => { + return Err(RelocationError::UnsupportedRelocationTarget { + symbol_kind, + relocation_number: rel_num, + }); + } }; (sym.section_index.unwrap(), address) } else { diff --git a/aya/src/maps/info.rs b/aya/src/maps/info.rs index 6a4bba13..90257b66 100644 --- a/aya/src/maps/info.rs +++ b/aya/src/maps/info.rs @@ -118,8 +118,12 @@ impl MapInfo { pub fn from_pin>(path: P) -> Result { use std::os::unix::ffi::OsStrExt as _; - // TODO: avoid this unwrap by adding a new error variant. - let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap(); + let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).map_err(|source| { + MapError::InvalidPath { + path: path.as_ref().to_path_buf(), + source, + } + })?; let fd = bpf_get_object(&path_string).map_err(|io_error| SyscallError { call: "BPF_OBJ_GET", io_error, diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 348838b3..50077b5e 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -49,13 +49,13 @@ //! implement the [Pod] trait. use std::{ borrow::Borrow, - ffi::CString, + ffi::{CString, NulError}, io, marker::PhantomData, mem, ops::Deref, os::fd::{AsFd, BorrowedFd, OwnedFd}, - path::Path, + path::{Path, PathBuf}, ptr, }; @@ -115,6 +115,19 @@ pub enum MapError { InvalidName { /// The map name name: String, + /// source error + #[source] + source: NulError, + }, + + /// Invalid Path encountered + #[error("invalid path `{path}`")] + InvalidPath { + /// The path + path: PathBuf, + /// source error + #[source] + source: NulError, }, /// Failed to create map @@ -575,8 +588,10 @@ impl MapData { name: &str, btf_fd: Option>, ) -> Result { - let c_name = CString::new(name) - .map_err(|std::ffi::NulError { .. }| MapError::InvalidName { name: name.into() })?; + let c_name = CString::new(name).map_err(|source| MapError::InvalidName { + name: name.into(), + source, + })?; // BPF_MAP_TYPE_PERF_EVENT_ARRAY's max_entries should not exceed the number of // CPUs. diff --git a/aya/src/programs/info.rs b/aya/src/programs/info.rs index 1a491b49..47aa4dc5 100644 --- a/aya/src/programs/info.rs +++ b/aya/src/programs/info.rs @@ -220,8 +220,12 @@ impl ProgramInfo { pub fn from_pin>(path: P) -> Result { use std::os::unix::ffi::OsStrExt as _; - // TODO: avoid this unwrap by adding a new error variant. - let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap(); + let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).map_err(|source| { + ProgramError::InvalidPath { + path: path.as_ref().to_path_buf(), + source, + } + })?; let fd = bpf_get_object(&path_string).map_err(|io_error| SyscallError { call: "BPF_OBJ_GET", io_error, diff --git a/aya/src/programs/links.rs b/aya/src/programs/links.rs index 1a80f9e1..78477c23 100644 --- a/aya/src/programs/links.rs +++ b/aya/src/programs/links.rs @@ -1,6 +1,6 @@ //! Program links. use std::{ - ffi::CString, + ffi::{CString, NulError}, io, os::fd::{AsFd as _, AsRawFd as _, BorrowedFd, RawFd}, path::{Path, PathBuf}, @@ -349,8 +349,12 @@ impl PinnedLink { pub fn from_pin>(path: P) -> Result { use std::os::unix::ffi::OsStrExt as _; - // TODO: avoid this unwrap by adding a new error variant. - let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap(); + let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).map_err(|source| { + LinkError::InvalidPath { + path: path.as_ref().to_path_buf(), + source, + } + })?; let fd = bpf_get_object(&path_string).map_err(|io_error| { LinkError::SyscallError(SyscallError { call: "BPF_OBJ_GET", @@ -594,6 +598,16 @@ pub enum LinkError { #[error("unknown link type {0}")] UnknownLinkType(u32), + /// Invalid path. + #[error("invalid path: {path}")] + InvalidPath { + /// path + path: PathBuf, + /// source error + #[source] + source: NulError, + }, + /// Syscall failed. #[error(transparent)] SyscallError(#[from] SyscallError), diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index 9e151ad8..a3e57df0 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -74,7 +74,7 @@ pub mod xdp; use std::{ borrow::Cow, - ffi::CString, + ffi::{CString, NulError}, io, os::fd::{AsFd, BorrowedFd}, path::{Path, PathBuf}, @@ -178,6 +178,16 @@ pub enum ProgramError { name: String, }, + /// The network interface name is invalid. + #[error("invalid network interface name {name}")] + InvalidInterfaceName { + /// interface name + name: String, + /// source error + #[source] + source: NulError, + }, + /// The program is not of the expected type. #[error("unexpected program type")] UnexpectedProgramType, @@ -218,11 +228,24 @@ pub enum ProgramError { #[error(transparent)] Btf(#[from] BtfError), - /// The program is not attached. + /// The program name is invalid. #[error("the program name `{name}` is invalid")] InvalidName { /// program name name: String, + /// source error + #[source] + source: NulError, + }, + + /// The path is invalid. + #[error("the path `{path}` is invalid")] + InvalidPath { + /// path + path: PathBuf, + /// source error + #[source] + source: NulError, }, /// An error occurred while working with IO. @@ -587,8 +610,12 @@ impl ProgramData { ) -> Result { use std::os::unix::ffi::OsStrExt as _; - // TODO: avoid this unwrap by adding a new error variant. - let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap(); + let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).map_err(|source| { + ProgramError::InvalidPath { + path: path.as_ref().to_path_buf(), + source, + } + })?; let fd = bpf_get_object(&path_string).map_err(|io_error| SyscallError { call: "bpf_obj_get", io_error, @@ -685,16 +712,15 @@ fn load_program( .unwrap_or(0) }); - let prog_name = if let Some(name) = name.as_deref() { - let prog_name = CString::new(name).map_err(|err @ std::ffi::NulError { .. }| { - let name = err.into_vec(); - let name = unsafe { String::from_utf8_unchecked(name) }; - ProgramError::InvalidName { name } - })?; - Some(prog_name) - } else { - None - }; + let prog_name = name + .as_deref() + .map(|name| { + CString::new(name).map_err(|source| { + let name = name.to_owned(); + ProgramError::InvalidName { name, source } + }) + }) + .transpose()?; let attr = EbpfLoadProgramAttrs { name: prog_name, diff --git a/aya/src/programs/raw_trace_point.rs b/aya/src/programs/raw_trace_point.rs index bd2e3946..5733a37c 100644 --- a/aya/src/programs/raw_trace_point.rs +++ b/aya/src/programs/raw_trace_point.rs @@ -51,7 +51,10 @@ impl RawTracePoint { /// /// The returned value can be used to detach, see [RawTracePoint::detach]. pub fn attach(&mut self, tp_name: &str) -> Result { - let tp_name_c = CString::new(tp_name).unwrap(); + let tp_name_c = CString::new(tp_name).map_err(|source| ProgramError::InvalidName { + name: tp_name.to_string(), + source, + })?; attach_raw_tracepoint(&mut self.data, Some(&tp_name_c)) } } diff --git a/aya/src/programs/tc.rs b/aya/src/programs/tc.rs index 58a0ecbf..1c434564 100644 --- a/aya/src/programs/tc.rs +++ b/aya/src/programs/tc.rs @@ -273,8 +273,7 @@ impl SchedClassifier { match options { TcAttachOptions::Netlink(options) => { let name = self.data.name.as_deref().unwrap_or_default(); - // TODO: avoid this unwrap by adding a new error variant. - let name = CString::new(name).unwrap(); + let name = CString::new(name).map_err(TcError::from)?; let (priority, handle) = unsafe { netlink_qdisc_attach( if_index as i32, diff --git a/aya/src/programs/xdp.rs b/aya/src/programs/xdp.rs index 8ba628f9..dc201cd4 100644 --- a/aya/src/programs/xdp.rs +++ b/aya/src/programs/xdp.rs @@ -104,8 +104,11 @@ impl Xdp { /// When `bpf_link_create` is unavailable or rejects the request, the call /// transparently falls back to the legacy netlink-based attach path. pub fn attach(&mut self, interface: &str, flags: XdpFlags) -> Result { - // TODO: avoid this unwrap by adding a new error variant. - let c_interface = CString::new(interface).unwrap(); + let c_interface = + CString::new(interface).map_err(|source| ProgramError::InvalidInterfaceName { + name: interface.to_string(), + source, + })?; let if_index = unsafe { libc::if_nametoindex(c_interface.as_ptr()) }; if if_index == 0 { return Err(ProgramError::UnknownInterface { diff --git a/xtask/public-api/aya-obj.txt b/xtask/public-api/aya-obj.txt index 8406f83a..4cb9663b 100644 --- a/xtask/public-api/aya-obj.txt +++ b/xtask/public-api/aya-obj.txt @@ -9225,6 +9225,9 @@ pub aya_obj::relocation::RelocationError::UnknownProgram::address: u64 pub aya_obj::relocation::RelocationError::UnknownProgram::section_index: usize pub aya_obj::relocation::RelocationError::UnknownSymbol pub aya_obj::relocation::RelocationError::UnknownSymbol::index: usize +pub aya_obj::relocation::RelocationError::UnsupportedRelocationTarget +pub aya_obj::relocation::RelocationError::UnsupportedRelocationTarget::relocation_number: u64 +pub aya_obj::relocation::RelocationError::UnsupportedRelocationTarget::symbol_kind: object::common::SymbolKind impl core::error::Error for aya_obj::relocation::RelocationError impl core::fmt::Debug for aya_obj::relocation::RelocationError pub fn aya_obj::relocation::RelocationError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index 27a51ec5..cb6a266f 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -1333,6 +1333,10 @@ pub aya::maps::MapError::InvalidMapType pub aya::maps::MapError::InvalidMapType::map_type: u32 pub aya::maps::MapError::InvalidName pub aya::maps::MapError::InvalidName::name: alloc::string::String +pub aya::maps::MapError::InvalidName::source: alloc::ffi::c_str::NulError +pub aya::maps::MapError::InvalidPath +pub aya::maps::MapError::InvalidPath::path: std::path::PathBuf +pub aya::maps::MapError::InvalidPath::source: alloc::ffi::c_str::NulError pub aya::maps::MapError::InvalidValueSize pub aya::maps::MapError::InvalidValueSize::expected: usize pub aya::maps::MapError::InvalidValueSize::size: usize @@ -4277,6 +4281,9 @@ impl core::convert::From for aya::programs::links::CgroupAttachMode pub fn aya::programs::links::CgroupAttachMode::from(t: T) -> T pub enum aya::programs::links::LinkError pub aya::programs::links::LinkError::InvalidLink +pub aya::programs::links::LinkError::InvalidPath +pub aya::programs::links::LinkError::InvalidPath::path: std::path::PathBuf +pub aya::programs::links::LinkError::InvalidPath::source: alloc::ffi::c_str::NulError pub aya::programs::links::LinkError::SyscallError(aya::sys::SyscallError) pub aya::programs::links::LinkError::UnknownLinkType(u32) impl core::convert::From for aya::programs::links::LinkError @@ -8207,8 +8214,15 @@ pub aya::programs::ProgramError::AttachCookieNotSupported pub aya::programs::ProgramError::Btf(aya_obj::btf::btf::BtfError) pub aya::programs::ProgramError::ExtensionError(aya::programs::extension::ExtensionError) pub aya::programs::ProgramError::IOError(std::io::error::Error) +pub aya::programs::ProgramError::InvalidInterfaceName +pub aya::programs::ProgramError::InvalidInterfaceName::name: alloc::string::String +pub aya::programs::ProgramError::InvalidInterfaceName::source: alloc::ffi::c_str::NulError pub aya::programs::ProgramError::InvalidName pub aya::programs::ProgramError::InvalidName::name: alloc::string::String +pub aya::programs::ProgramError::InvalidName::source: alloc::ffi::c_str::NulError +pub aya::programs::ProgramError::InvalidPath +pub aya::programs::ProgramError::InvalidPath::path: std::path::PathBuf +pub aya::programs::ProgramError::InvalidPath::source: alloc::ffi::c_str::NulError pub aya::programs::ProgramError::KProbeError(aya::programs::kprobe::KProbeError) pub aya::programs::ProgramError::LoadError pub aya::programs::ProgramError::LoadError::io_error: std::io::error::Error