From 572d047e37111b732be49ef3ad6fb16f70aa4063 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 11 Aug 2023 10:20:38 -0400 Subject: [PATCH 1/2] test: avoid lossy string conversions We can be strict in tests. --- aya-obj/src/obj.rs | 43 ++++++++++++++++++------------------------ aya/src/sys/netlink.rs | 2 +- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/aya-obj/src/obj.rs b/aya-obj/src/obj.rs index 70375df2..73d87e68 100644 --- a/aya-obj/src/obj.rs +++ b/aya-obj/src/obj.rs @@ -1642,15 +1642,12 @@ mod tests { let prog_foo = obj.programs.get("foo").unwrap(); - assert_matches!( - prog_foo, - Program { - license, - kernel_version: None, - section: ProgramSection::KProbe { .. }, - .. - } if license.to_str().unwrap() == "GPL" - ); + assert_matches!(prog_foo, Program { + license, + kernel_version: None, + section: ProgramSection::KProbe { .. }, + .. + } => assert_eq!(license.to_str().unwrap(), "GPL")); assert_matches!( obj.functions.get(&prog_foo.function_key()), @@ -1704,14 +1701,12 @@ mod tests { let prog_bar = obj.programs.get("bar").unwrap(); let function_bar = obj.functions.get(&prog_bar.function_key()).unwrap(); - assert_matches!(prog_foo, - Program { - license, - kernel_version: None, - section: ProgramSection::KProbe { .. }, - .. - } if license.to_string_lossy() == "GPL" - ); + assert_matches!(prog_foo, Program { + license, + kernel_version: None, + section: ProgramSection::KProbe { .. }, + .. + } => assert_eq!(license.to_str().unwrap(), "GPL")); assert_matches!( function_foo, Function { @@ -1724,14 +1719,12 @@ mod tests { } if name == "foo" && instructions.len() == 1 ); - assert_matches!(prog_bar, - Program { - license, - kernel_version: None, - section: ProgramSection::KProbe { .. }, - .. - } if license.to_string_lossy() == "GPL" - ); + assert_matches!(prog_bar, Program { + license, + kernel_version: None, + section: ProgramSection::KProbe { .. }, + .. + } => assert_eq!(license.to_str().unwrap(), "GPL")); assert_matches!( function_bar, Function { diff --git a/aya/src/sys/netlink.rs b/aya/src/sys/netlink.rs index ac302f50..a7b24902 100644 --- a/aya/src/sys/netlink.rs +++ b/aya/src/sys/netlink.rs @@ -742,6 +742,6 @@ mod tests { TCA_BPF_NAME as u16 ); let name = CStr::from_bytes_with_nul(inner.data).unwrap(); - assert_eq!(name.to_string_lossy(), "foo"); + assert_eq!(name.to_str().unwrap(), "foo"); } } From 0bba9b14b02a01ca33dbb1fa4a910b77a73a4d65 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 10 Aug 2023 11:44:37 -0400 Subject: [PATCH 2/2] maps,programs: avoid path UTF-8 assumptions --- aya/src/bpf.rs | 12 -- aya/src/maps/mod.rs | 41 ++++-- aya/src/pin.rs | 10 +- aya/src/programs/kprobe.rs | 10 +- aya/src/programs/links.rs | 22 +-- aya/src/programs/mod.rs | 23 +-- aya/src/programs/probe.rs | 243 ++++++++++++++++++++++---------- aya/src/programs/tc.rs | 11 +- aya/src/programs/trace_point.rs | 4 +- aya/src/programs/uprobe.rs | 132 +++++++++-------- aya/src/programs/xdp.rs | 1 + aya/src/sys/perf_event.rs | 7 +- xtask/public-api/aya.txt | 13 +- 13 files changed, 325 insertions(+), 204 deletions(-) diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index f8bf467c..f967c737 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -1,7 +1,6 @@ use std::{ borrow::Cow, collections::{HashMap, HashSet}, - ffi::CString, fs, io, os::{ fd::{AsFd as _, OwnedFd}, @@ -608,10 +607,6 @@ impl<'a> BpfLoader<'a> { ProgramSection::SchedClassifier => { Program::SchedClassifier(SchedClassifier { data: ProgramData::new(prog_name, obj, btf_fd, *verifier_log_level), - name: unsafe { - CString::from_vec_unchecked(Vec::from(name.clone())) - .into_boxed_c_str() - }, }) } ProgramSection::CgroupSkb => Program::CgroupSkb(CgroupSkb { @@ -960,13 +955,6 @@ pub enum BpfError { name: u32, }, - /// Invalid path - #[error("invalid path `{error}`")] - InvalidPath { - /// The error message - error: String, - }, - /// Error parsing BPF object #[error("error parsing BPF object: {0}")] ParseError(#[from] ParseError), diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 0cd5386e..dfa0949c 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -515,9 +515,19 @@ impl MapData { name: &str, btf_fd: Option>, ) -> Result { + use std::os::unix::ffi::OsStrExt as _; + // try to open map in case it's already pinned - let map_path = path.as_ref().join(name); - let path_string = CString::new(map_path.to_str().unwrap()).unwrap(); + let path = path.as_ref().join(name); + let path_string = match CString::new(path.as_os_str().as_bytes()) { + Ok(path) => path, + Err(error) => { + return Err(MapError::PinError { + name: Some(name.into()), + error: PinError::InvalidPinPath { path, error }, + }); + } + }; match bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_GET", io_error, @@ -540,14 +550,16 @@ impl MapData { /// Loads a map from a pinned path in bpffs. pub fn from_pin>(path: P) -> Result { + use std::os::unix::ffi::OsStrExt as _; + + let path = path.as_ref(); let path_string = - CString::new(path.as_ref().to_string_lossy().into_owned()).map_err(|e| { - MapError::PinError { - name: None, - error: PinError::InvalidPinPath { - error: e.to_string(), - }, - } + CString::new(path.as_os_str().as_bytes()).map_err(|error| MapError::PinError { + name: None, + error: PinError::InvalidPinPath { + path: path.into(), + error, + }, })?; let fd = bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError { @@ -580,16 +592,15 @@ impl MapData { } pub(crate) fn pin>(&mut self, name: &str, path: P) -> Result<(), PinError> { + use std::os::unix::ffi::OsStrExt as _; + let Self { fd, pinned, obj: _ } = self; if *pinned { return Err(PinError::AlreadyPinned { name: name.into() }); } - let map_path = path.as_ref().join(name); - let path_string = CString::new(map_path.to_string_lossy().into_owned()).map_err(|e| { - PinError::InvalidPinPath { - error: e.to_string(), - } - })?; + let path = path.as_ref().join(name); + let path_string = CString::new(path.as_os_str().as_bytes()) + .map_err(|error| PinError::InvalidPinPath { path, error })?; bpf_pin_object(*fd, &path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_PIN", io_error, diff --git a/aya/src/pin.rs b/aya/src/pin.rs index eda85bbc..743df95f 100644 --- a/aya/src/pin.rs +++ b/aya/src/pin.rs @@ -19,10 +19,14 @@ pub enum PinError { name: String, }, /// The path for the BPF object is not valid. - #[error("invalid pin path `{error}`")] + #[error("invalid pin path `{}`", path.display())] InvalidPinPath { - /// The error message. - error: String, + /// The path. + path: std::path::PathBuf, + + #[source] + /// The source error. + error: std::ffi::NulError, }, /// An error ocurred making a syscall. #[error(transparent)] diff --git a/aya/src/programs/kprobe.rs b/aya/src/programs/kprobe.rs index a0c8d2b4..ef34a3f4 100644 --- a/aya/src/programs/kprobe.rs +++ b/aya/src/programs/kprobe.rs @@ -1,5 +1,9 @@ //! Kernel space probes. -use std::{io, os::fd::AsFd as _, path::Path}; +use std::{ + io, + os::fd::AsFd as _, + path::{Path, PathBuf}, +}; use thiserror::Error; use crate::{ @@ -68,7 +72,7 @@ impl KProbe { /// /// The returned value can be used to detach from the given function, see [KProbe::detach]. pub fn attach(&mut self, fn_name: &str, offset: u64) -> Result { - attach(&mut self.data, self.kind, fn_name, offset, None) + attach(&mut self.data, self.kind, Path::new(fn_name), offset, None) } /// Detaches the program. @@ -114,7 +118,7 @@ pub enum KProbeError { #[error("`{filename}`")] FileError { /// The file name - filename: String, + filename: PathBuf, /// The [`io::Error`] returned from the file operation #[source] io_error: io::Error, diff --git a/aya/src/programs/links.rs b/aya/src/programs/links.rs index 77341adc..71b0aba1 100644 --- a/aya/src/programs/links.rs +++ b/aya/src/programs/links.rs @@ -146,19 +146,22 @@ impl FdLink { /// # Ok::<(), Error>(()) /// ``` pub fn pin>(self, path: P) -> Result { - let path_string = - CString::new(path.as_ref().to_string_lossy().into_owned()).map_err(|e| { - PinError::InvalidPinPath { - error: e.to_string(), - } - })?; + use std::os::unix::ffi::OsStrExt as _; + + let path = path.as_ref(); + let path_string = CString::new(path.as_os_str().as_bytes()).map_err(|error| { + PinError::InvalidPinPath { + path: path.into(), + error, + } + })?; bpf_pin_object(self.fd.as_raw_fd(), &path_string).map_err(|(_, io_error)| { SyscallError { call: "BPF_OBJ_PIN", io_error, } })?; - Ok(PinnedLink::new(PathBuf::from(path.as_ref()), self)) + Ok(PinnedLink::new(path.into(), self)) } } @@ -203,7 +206,10 @@ impl PinnedLink { /// Creates a [`crate::programs::links::PinnedLink`] from a valid path on bpffs. pub fn from_pin>(path: P) -> Result { - let path_string = CString::new(path.as_ref().to_string_lossy().to_string()).unwrap(); + 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 fd = bpf_get_object(&path_string).map_err(|(_, io_error)| { LinkError::SyscallError(SyscallError { call: "BPF_OBJ_GET", diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index dcec8bf3..07755307 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -487,9 +487,10 @@ impl ProgramData { path: P, verifier_log_level: VerifierLogLevel, ) -> Result { - let path_string = - CString::new(path.as_ref().as_os_str().to_string_lossy().as_bytes()).unwrap(); + 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 fd = bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError { call: "bpf_obj_get", io_error, @@ -520,6 +521,8 @@ fn unload_program(data: &mut ProgramData) -> Result<(), ProgramError } fn pin_program>(data: &ProgramData, path: P) -> Result<(), PinError> { + use std::os::unix::ffi::OsStrExt as _; + let fd = data.fd.as_ref().ok_or(PinError::NoFd { name: data .name @@ -527,11 +530,12 @@ fn pin_program>(data: &ProgramData, path: P) -> Resul .unwrap_or("") .to_string(), })?; - let path_string = CString::new(path.as_ref().to_string_lossy().into_owned()).map_err(|e| { - PinError::InvalidPinPath { - error: e.to_string(), - } - })?; + let path = path.as_ref(); + let path_string = + CString::new(path.as_os_str().as_bytes()).map_err(|error| PinError::InvalidPinPath { + path: path.into(), + error, + })?; bpf_pin_object(fd.as_fd().as_raw_fd(), &path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_PIN", io_error, @@ -1073,7 +1077,10 @@ impl ProgramInfo { /// Loads a program from a pinned path in bpffs. pub fn from_pin>(path: P) -> Result { - let path_string = CString::new(path.as_ref().to_str().unwrap()).unwrap(); + 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 fd = bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_GET", io_error, diff --git a/aya/src/programs/probe.rs b/aya/src/programs/probe.rs index bff48612..ea0d59b5 100644 --- a/aya/src/programs/probe.rs +++ b/aya/src/programs/probe.rs @@ -1,10 +1,12 @@ use crate::util::KernelVersion; use libc::pid_t; use std::{ + ffi::{OsStr, OsString}, + fmt::Write as _, fs::{self, OpenOptions}, io::{self, Write}, os::fd::{AsFd as _, AsRawFd as _, OwnedFd}, - path::Path, + path::{Path, PathBuf}, process, sync::atomic::{AtomicUsize, Ordering}, }; @@ -42,16 +44,64 @@ impl ProbeKind { } } +pub(crate) fn lines(bytes: &[u8]) -> impl Iterator { + use std::os::unix::ffi::OsStrExt as _; + + bytes.as_ref().split(|b| b == &b'\n').map(|mut line| { + while let [stripped @ .., c] = line { + if c.is_ascii_whitespace() { + line = stripped; + continue; + } + break; + } + OsStr::from_bytes(line) + }) +} + +pub(crate) trait OsStringExt { + fn starts_with(&self, needle: &OsStr) -> bool; + fn ends_with(&self, needle: &OsStr) -> bool; + fn strip_prefix(&self, prefix: &OsStr) -> Option<&OsStr>; + fn strip_suffix(&self, suffix: &OsStr) -> Option<&OsStr>; +} + +impl OsStringExt for OsStr { + fn starts_with(&self, needle: &OsStr) -> bool { + use std::os::unix::ffi::OsStrExt as _; + self.as_bytes().starts_with(needle.as_bytes()) + } + + fn ends_with(&self, needle: &OsStr) -> bool { + use std::os::unix::ffi::OsStrExt as _; + self.as_bytes().ends_with(needle.as_bytes()) + } + + fn strip_prefix(&self, prefix: &OsStr) -> Option<&OsStr> { + use std::os::unix::ffi::OsStrExt as _; + self.as_bytes() + .strip_prefix(prefix.as_bytes()) + .map(Self::from_bytes) + } + + fn strip_suffix(&self, suffix: &OsStr) -> Option<&OsStr> { + use std::os::unix::ffi::OsStrExt as _; + self.as_bytes() + .strip_suffix(suffix.as_bytes()) + .map(Self::from_bytes) + } +} + #[derive(Debug)] pub(crate) struct ProbeEvent { kind: ProbeKind, - event_alias: String, + event_alias: OsString, } pub(crate) fn attach>( program_data: &mut ProgramData, kind: ProbeKind, - fn_name: &str, + fn_name: &Path, offset: u64, pid: Option, ) -> Result { @@ -90,7 +140,7 @@ pub(crate) fn detach_debug_fs(event: ProbeEvent) -> Result<(), ProgramError> { fn create_as_probe( kind: ProbeKind, - fn_name: &str, + fn_name: &Path, offset: u64, pid: Option, ) -> Result { @@ -126,10 +176,10 @@ fn create_as_probe( fn create_as_trace_point( kind: ProbeKind, - name: &str, + name: &Path, offset: u64, pid: Option, -) -> Result<(OwnedFd, String), ProgramError> { +) -> Result<(OwnedFd, OsString), ProgramError> { use ProbeKind::*; let tracefs = find_tracefs_path()?; @@ -142,7 +192,7 @@ fn create_as_trace_point( }; let category = format!("{}s", kind.pmu()); - let tpid = read_sys_fs_trace_point_id(tracefs, &category, &event_alias)?; + let tpid = read_sys_fs_trace_point_id(tracefs, &category, event_alias.as_ref())?; let fd = perf_event_open_trace_point(tpid, pid).map_err(|(_code, io_error)| SyscallError { call: "perf_event_open", io_error, @@ -154,9 +204,10 @@ fn create_as_trace_point( fn create_probe_event( tracefs: &Path, kind: ProbeKind, - fn_name: &str, + fn_name: &Path, offset: u64, -) -> Result { +) -> Result { + use std::os::unix::ffi::OsStrExt as _; use ProbeKind::*; let events_file_name = tracefs.join(format!("{}_events", kind.pmu())); @@ -165,93 +216,129 @@ fn create_probe_event( KRetProbe | URetProbe => 'r', }; - let fixed_fn_name = fn_name.replace(['.', '/', '-'], "_"); + let fn_name = fn_name.as_os_str(); - let event_alias = format!( - "aya_{}_{}_{}_{:#x}_{}", + let mut event_alias = OsString::new(); + write!( + &mut event_alias, + "aya_{}_{}_", process::id(), probe_type_prefix, - fixed_fn_name, + ) + .unwrap(); + for b in fn_name.as_bytes() { + let b = match *b { + b'.' | b'/' | b'-' => b'_', + b => b, + }; + event_alias.push(OsStr::from_bytes(&[b])); + } + write!( + &mut event_alias, + "_{:#x}_{}", offset, PROBE_NAME_INDEX.fetch_add(1, Ordering::AcqRel) - ); - let offset_suffix = match kind { - KProbe => format!("+{offset}"), - UProbe | URetProbe => format!(":{offset:#x}"), - _ => String::new(), + ) + .unwrap(); + + let mut probe = OsString::new(); + write!(&mut probe, "{}:{}s/", probe_type_prefix, kind.pmu(),).unwrap(); + probe.push(&event_alias); + probe.push(" "); + probe.push(fn_name); + match kind { + KProbe => write!(&mut probe, "+{offset}").unwrap(), + UProbe | URetProbe => write!(&mut probe, ":{offset:#x}").unwrap(), + _ => {} }; - let probe = format!( - "{}:{}s/{} {}{}\n", - probe_type_prefix, - kind.pmu(), - event_alias, - fn_name, - offset_suffix - ); + probe.push("\n"); - let mut events_file = OpenOptions::new() + OpenOptions::new() .append(true) .open(&events_file_name) - .map_err(|e| (events_file_name.display().to_string(), e))?; - - events_file - .write_all(probe.as_bytes()) - .map_err(|e| (events_file_name.display().to_string(), e))?; + .and_then(|mut events_file| events_file.write_all(probe.as_bytes())) + .map_err(|e| (events_file_name, e))?; Ok(event_alias) } -fn delete_probe_event(tracefs: &Path, event: ProbeEvent) -> Result<(), (String, io::Error)> { +fn delete_probe_event(tracefs: &Path, event: ProbeEvent) -> Result<(), (PathBuf, io::Error)> { + use std::os::unix::ffi::OsStrExt as _; + let ProbeEvent { kind, event_alias } = event; let events_file_name = tracefs.join(format!("{}_events", kind.pmu())); - let events = fs::read_to_string(&events_file_name) - .map_err(|e| (events_file_name.display().to_string(), e))?; - - let found = events.lines().any(|line| line.contains(&event_alias)); - - if found { - let mut events_file = OpenOptions::new() - .append(true) - .open(&events_file_name) - .map_err(|e| (events_file_name.display().to_string(), e))?; - - let rm = format!("-:{event_alias}\n"); - - events_file - .write_all(rm.as_bytes()) - .map_err(|e| (events_file_name.display().to_string(), e))?; - } - - Ok(()) + fs::read(&events_file_name) + .and_then(|events| { + let found = lines(&events).any(|line| { + let mut line = line.as_bytes(); + // See [`create_probe_event`] and the documentation: + // + // https://docs.kernel.org/trace/kprobetrace.html + // + // https://docs.kernel.org/trace/uprobetracer.html + loop { + match line.split_first() { + None => break false, + Some((b, rest)) => { + line = rest; + if *b == b'/' { + break line.starts_with(event_alias.as_bytes()); + } + } + } + } + }); + + if found { + OpenOptions::new() + .append(true) + .open(&events_file_name) + .and_then(|mut events_file| { + let mut rm = OsString::new(); + rm.push("-:"); + rm.push(event_alias); + rm.push("\n"); + + events_file.write_all(rm.as_bytes()) + }) + } else { + Ok(()) + } + }) + .map_err(|e| (events_file_name, e)) } -fn read_sys_fs_perf_type(pmu: &str) -> Result { - let file = format!("/sys/bus/event_source/devices/{pmu}/type"); - - let perf_ty = fs::read_to_string(&file).map_err(|e| (file.clone(), e))?; - let perf_ty = perf_ty - .trim() - .parse::() - .map_err(|e| (file, io::Error::new(io::ErrorKind::Other, e)))?; - - Ok(perf_ty) +fn read_sys_fs_perf_type(pmu: &str) -> Result { + let file = Path::new("/sys/bus/event_source/devices") + .join(pmu) + .join("type"); + + fs::read_to_string(&file) + .and_then(|perf_ty| { + perf_ty + .trim() + .parse::() + .map_err(|e| io::Error::new(io::ErrorKind::Other, e)) + }) + .map_err(|e| (file, e)) } -fn read_sys_fs_perf_ret_probe(pmu: &str) -> Result { - let file = format!("/sys/bus/event_source/devices/{pmu}/format/retprobe"); - - let data = fs::read_to_string(&file).map_err(|e| (file.clone(), e))?; - - let mut parts = data.trim().splitn(2, ':').skip(1); - let config = parts.next().ok_or_else(|| { - ( - file.clone(), - io::Error::new(io::ErrorKind::Other, "invalid format"), - ) - })?; - - config - .parse::() - .map_err(|e| (file, io::Error::new(io::ErrorKind::Other, e))) +fn read_sys_fs_perf_ret_probe(pmu: &str) -> Result { + let file = Path::new("/sys/bus/event_source/devices") + .join(pmu) + .join("format/retprobe"); + + fs::read_to_string(&file) + .and_then(|data| { + let mut parts = data.trim().splitn(2, ':').skip(1); + let config = parts + .next() + .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid format"))?; + + config + .parse::() + .map_err(|e| io::Error::new(io::ErrorKind::Other, e)) + }) + .map_err(|e| (file, e)) } diff --git a/aya/src/programs/tc.rs b/aya/src/programs/tc.rs index 59a32ab2..8cb73cea 100644 --- a/aya/src/programs/tc.rs +++ b/aya/src/programs/tc.rs @@ -74,7 +74,6 @@ pub enum TcAttachType { #[doc(alias = "BPF_PROG_TYPE_SCHED_CLS")] pub struct SchedClassifier { pub(crate) data: ProgramData, - pub(crate) name: Box, } /// Errors from TC programs @@ -158,12 +157,15 @@ impl SchedClassifier { let prog_fd = prog_fd.as_raw_fd(); let if_index = ifindex_from_ifname(interface) .map_err(|io_error| TcError::NetlinkError { io_error })?; + 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 (priority, handle) = unsafe { netlink_qdisc_attach( if_index as i32, &attach_type, prog_fd, - &self.name, + &name, options.priority, options.handle, ) @@ -204,10 +206,7 @@ impl SchedClassifier { /// the program being unloaded from the kernel if it is still pinned. pub fn from_pin>(path: P) -> Result { let data = ProgramData::from_pinned_path(path, VerifierLogLevel::default())?; - let cname = CString::new(data.name.clone().unwrap_or_default()) - .unwrap() - .into_boxed_c_str(); - Ok(Self { data, name: cname }) + Ok(Self { data }) } } diff --git a/aya/src/programs/trace_point.rs b/aya/src/programs/trace_point.rs index 8ed34eb4..b9a4c5b1 100644 --- a/aya/src/programs/trace_point.rs +++ b/aya/src/programs/trace_point.rs @@ -86,7 +86,7 @@ impl TracePoint { let prog_fd = prog_fd.as_fd(); let prog_fd = prog_fd.as_raw_fd(); let tracefs = find_tracefs_path()?; - let id = read_sys_fs_trace_point_id(tracefs, category, name)?; + let id = read_sys_fs_trace_point_id(tracefs, category, name.as_ref())?; let fd = perf_event_open_trace_point(id, None).map_err(|(_code, io_error)| SyscallError { call: "perf_event_open_trace_point", @@ -149,7 +149,7 @@ impl TryFrom for TracePointLink { pub(crate) fn read_sys_fs_trace_point_id( tracefs: &Path, category: &str, - name: &str, + name: &Path, ) -> Result { let file = tracefs.join("events").join(category).join(name).join("id"); diff --git a/aya/src/programs/uprobe.rs b/aya/src/programs/uprobe.rs index 5e30d174..00d306ad 100644 --- a/aya/src/programs/uprobe.rs +++ b/aya/src/programs/uprobe.rs @@ -4,7 +4,7 @@ use object::{Object, ObjectSection, ObjectSymbol}; use std::{ borrow::Cow, error::Error, - ffi::CStr, + ffi::{CStr, OsStr, OsString}, fs, io::{self, BufRead, Cursor, Read}, mem, @@ -19,7 +19,7 @@ use crate::{ programs::{ define_link_wrapper, load_program, perf_attach::{PerfLinkIdInner, PerfLinkInner}, - probe::{attach, ProbeKind}, + probe::{attach, OsStringExt as _, ProbeKind}, FdLink, LinkError, ProgramData, ProgramError, }, sys::bpf_link_get_info_by_fd, @@ -83,7 +83,7 @@ impl UProbe { target: T, pid: Option, ) -> Result { - let path = resolve_attach_path(&target, pid)?; + let path = resolve_attach_path(target.as_ref(), pid)?; let sym_offset = if let Some(fn_name) = fn_name { resolve_symbol(&path, fn_name).map_err(|error| UProbeError::SymbolError { @@ -124,37 +124,33 @@ impl UProbe { } } -fn resolve_attach_path>( - target: &T, - pid: Option, -) -> Result, UProbeError> { +fn resolve_attach_path(target: &Path, pid: Option) -> Result, UProbeError> { // Look up the path for the target. If it there is a pid, and the target is a library name // that is in the process's memory map, use the path of that library. Otherwise, use the target as-is. - let target = target.as_ref(); - let invalid_target = || UProbeError::InvalidTarget { - path: target.to_owned(), - }; - let target_str = target.to_str().ok_or_else(invalid_target)?; pid.and_then(|pid| { - find_lib_in_proc_maps(pid, target_str) + find_lib_in_proc_maps(pid, target) .map_err(|io_error| UProbeError::FileError { - filename: format!("/proc/{pid}/maps"), + filename: Path::new("/proc").join(pid.to_string()).join("maps"), io_error, }) .map(|v| v.map(Cow::Owned)) .transpose() }) - .or_else(|| target.is_absolute().then(|| Ok(Cow::Borrowed(target_str)))) + .or_else(|| target.is_absolute().then(|| Ok(Cow::Borrowed(target)))) .or_else(|| { LD_SO_CACHE .as_ref() .map_err(|error| UProbeError::InvalidLdSoCache { io_error: error.clone(), }) - .map(|cache| cache.resolve(target_str).map(Cow::Borrowed)) + .map(|cache| cache.resolve(target).map(Cow::Borrowed)) .transpose() }) - .unwrap_or_else(|| Err(invalid_target())) + .unwrap_or_else(|| { + Err(UProbeError::InvalidTarget { + path: target.to_owned(), + }) + }) } // Only run this test on linux with glibc because only in that configuration do we know that we'll @@ -171,7 +167,8 @@ fn test_resolve_attach_path() { // Now let's resolve the path to libc. It should exist in the current process's memory map and // then in the ld.so.cache. - let libc_path = resolve_attach_path(&"libc", Some(pid)).unwrap(); + let libc_path = resolve_attach_path("libc".as_ref(), Some(pid)).unwrap(); + let libc_path = libc_path.to_str().unwrap(); // Make sure we got a path that contains libc. assert!(libc_path.contains("libc"), "libc_path: {}", libc_path); @@ -242,52 +239,60 @@ pub enum UProbeError { #[error("`{filename}`")] FileError { /// The file name - filename: String, + filename: PathBuf, /// The [`io::Error`] returned from the file operation #[source] io_error: io::Error, }, } -fn proc_maps_libs(pid: pid_t) -> Result, io::Error> { +fn proc_maps_libs(pid: pid_t) -> Result, io::Error> { + use std::os::unix::ffi::OsStrExt as _; + let maps_file = format!("/proc/{pid}/maps"); - let data = fs::read_to_string(maps_file)?; - - Ok(data - .lines() - .filter_map(|line| { - let line = line.split_whitespace().last()?; - if line.starts_with('/') { - let path = PathBuf::from(line); - let key = path.file_name().unwrap().to_string_lossy().into_owned(); - Some((key, path.to_string_lossy().to_string())) - } else { - None + let data = fs::read(maps_file)?; + + let libs = data + .split(|b| b == &b'\n') + .filter_map(|mut line| { + while let [stripped @ .., c] = line { + if c.is_ascii_whitespace() { + line = stripped; + continue; + } + break; } + let path = line.split(|b| b.is_ascii_whitespace()).last()?; + let path = Path::new(OsStr::from_bytes(path)); + path.is_absolute() + .then(|| { + path.file_name() + .map(|file_name| (file_name.to_owned(), path.to_owned())) + }) + .flatten() }) - .collect()) + .collect(); + Ok(libs) } -fn find_lib_in_proc_maps(pid: pid_t, lib: &str) -> Result, io::Error> { +fn find_lib_in_proc_maps(pid: pid_t, lib: &Path) -> Result, io::Error> { let libs = proc_maps_libs(pid)?; - let ret = if lib.contains(".so") { - libs.into_iter().find(|(k, _)| k.as_str().starts_with(lib)) - } else { - libs.into_iter().find(|(k, _)| { - k.strip_prefix(lib) - .map(|k| k.starts_with(".so") || k.starts_with('-')) - .unwrap_or_default() - }) - }; + let lib = lib.as_os_str(); + let lib = lib.strip_suffix(OsStr::new(".so")).unwrap_or(lib); - Ok(ret.map(|(_, v)| v)) + Ok(libs.into_iter().find_map(|(file_name, path)| { + file_name.strip_prefix(lib).and_then(|suffix| { + (suffix.starts_with(OsStr::new(".so")) || suffix.starts_with(OsStr::new("-"))) + .then_some(path) + }) + })) } #[derive(Debug)] pub(crate) struct CacheEntry { - key: String, - value: String, + key: OsString, + value: OsString, _flags: i32, } @@ -368,11 +373,16 @@ impl LdSoCache { } let read_str = |pos| { - unsafe { - CStr::from_ptr(cursor.get_ref()[offset + pos..].as_ptr() as *const c_char) - } - .to_string_lossy() - .into_owned() + use std::os::unix::ffi::OsStrExt as _; + OsStr::from_bytes( + unsafe { + CStr::from_ptr( + cursor.get_ref()[offset + pos..].as_ptr() as *const c_char + ) + } + .to_bytes(), + ) + .to_owned() }; let key = read_str(k_pos); @@ -389,16 +399,18 @@ impl LdSoCache { Ok(Self { entries }) } - fn resolve(&self, lib: &str) -> Option<&str> { - let lib = if !lib.contains(".so") { - lib.to_string() + ".so" - } else { - lib.to_string() - }; + fn resolve(&self, lib: &Path) -> Option<&Path> { + let lib = lib.as_os_str(); + let lib = lib.strip_suffix(OsStr::new(".so")).unwrap_or(lib); self.entries .iter() - .find(|entry| entry.key.starts_with(&lib)) - .map(|entry| entry.value.as_str()) + .find_map(|CacheEntry { key, value, _flags }| { + key.strip_prefix(lib).and_then(|suffix| { + suffix + .starts_with(OsStr::new(".so")) + .then_some(Path::new(value.as_os_str())) + }) + }) } } @@ -420,7 +432,7 @@ enum ResolveSymbolError { SectionFileRangeNone(String, Result), } -fn resolve_symbol(path: &str, symbol: &str) -> Result { +fn resolve_symbol(path: &Path, symbol: &str) -> Result { let data = fs::read(path)?; let obj = object::read::File::parse(&*data)?; diff --git a/aya/src/programs/xdp.rs b/aya/src/programs/xdp.rs index 14db680c..22eb4f70 100644 --- a/aya/src/programs/xdp.rs +++ b/aya/src/programs/xdp.rs @@ -100,6 +100,7 @@ impl Xdp { /// [`XdpError::NetlinkError`] is returned for older /// kernels. 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 if_index = unsafe { if_nametoindex(c_interface.as_ptr()) }; if if_index == 0 { diff --git a/aya/src/sys/perf_event.rs b/aya/src/sys/perf_event.rs index dad86a45..000fecbc 100644 --- a/aya/src/sys/perf_event.rs +++ b/aya/src/sys/perf_event.rs @@ -2,6 +2,7 @@ use std::{ ffi::{c_long, CString}, io, mem, os::fd::{BorrowedFd, FromRawFd as _, OwnedFd}, + path::Path, }; use libc::{c_int, pid_t}; @@ -62,17 +63,19 @@ pub(crate) fn perf_event_open_bpf(cpu: c_int) -> SysResult { pub(crate) fn perf_event_open_probe( ty: u32, ret_bit: Option, - name: &str, + name: &Path, offset: u64, pid: Option, ) -> SysResult { + use std::os::unix::ffi::OsStrExt as _; + let mut attr = unsafe { mem::zeroed::() }; if let Some(ret_bit) = ret_bit { attr.config = 1 << ret_bit; } - let c_name = CString::new(name).unwrap(); + let c_name = CString::new(name.as_os_str().as_bytes()).unwrap(); attr.size = mem::size_of::() as u32; attr.type_ = ty; diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index d33436fb..1ef6c0c8 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -1752,7 +1752,8 @@ pub enum aya::pin::PinError pub aya::pin::PinError::AlreadyPinned pub aya::pin::PinError::AlreadyPinned::name: alloc::string::String pub aya::pin::PinError::InvalidPinPath -pub aya::pin::PinError::InvalidPinPath::error: alloc::string::String +pub aya::pin::PinError::InvalidPinPath::error: alloc::ffi::c_str::NulError +pub aya::pin::PinError::InvalidPinPath::path: std::path::PathBuf pub aya::pin::PinError::NoFd pub aya::pin::PinError::NoFd::name: alloc::string::String pub aya::pin::PinError::SyscallError(crate::sys::SyscallError) @@ -2860,7 +2861,7 @@ pub fn aya::programs::fexit::FExitLinkId::from(t: T) -> T pub mod aya::programs::kprobe pub enum aya::programs::kprobe::KProbeError pub aya::programs::kprobe::KProbeError::FileError -pub aya::programs::kprobe::KProbeError::FileError::filename: alloc::string::String +pub aya::programs::kprobe::KProbeError::FileError::filename: std::path::PathBuf pub aya::programs::kprobe::KProbeError::FileError::io_error: std::io::error::Error impl core::convert::From for aya::programs::ProgramError pub fn aya::programs::ProgramError::from(source: aya::programs::kprobe::KProbeError) -> Self @@ -4330,7 +4331,7 @@ pub fn aya::programs::trace_point::TracePointLinkId::from(t: T) -> T pub mod aya::programs::uprobe pub enum aya::programs::uprobe::UProbeError pub aya::programs::uprobe::UProbeError::FileError -pub aya::programs::uprobe::UProbeError::FileError::filename: alloc::string::String +pub aya::programs::uprobe::UProbeError::FileError::filename: std::path::PathBuf pub aya::programs::uprobe::UProbeError::FileError::io_error: std::io::error::Error pub aya::programs::uprobe::UProbeError::InvalidLdSoCache pub aya::programs::uprobe::UProbeError::InvalidLdSoCache::io_error: alloc::sync::Arc @@ -4814,7 +4815,7 @@ impl core::convert::From for aya::programs::extension::ExtensionError pub fn aya::programs::extension::ExtensionError::from(t: T) -> T pub enum aya::programs::KProbeError pub aya::programs::KProbeError::FileError -pub aya::programs::KProbeError::FileError::filename: alloc::string::String +pub aya::programs::KProbeError::FileError::filename: std::path::PathBuf pub aya::programs::KProbeError::FileError::io_error: std::io::error::Error impl core::convert::From for aya::programs::ProgramError pub fn aya::programs::ProgramError::from(source: aya::programs::kprobe::KProbeError) -> Self @@ -5442,7 +5443,7 @@ impl core::convert::From for aya::programs::trace_point::TracePointError pub fn aya::programs::trace_point::TracePointError::from(t: T) -> T pub enum aya::programs::UProbeError pub aya::programs::UProbeError::FileError -pub aya::programs::UProbeError::FileError::filename: alloc::string::String +pub aya::programs::UProbeError::FileError::filename: std::path::PathBuf pub aya::programs::UProbeError::FileError::io_error: std::io::error::Error pub aya::programs::UProbeError::InvalidLdSoCache pub aya::programs::UProbeError::InvalidLdSoCache::io_error: alloc::sync::Arc @@ -6988,8 +6989,6 @@ pub aya::BpfError::BtfRelocationError(aya_obj::btf::relocation::BtfRelocationErr pub aya::BpfError::FileError pub aya::BpfError::FileError::error: std::io::error::Error pub aya::BpfError::FileError::path: std::path::PathBuf -pub aya::BpfError::InvalidPath -pub aya::BpfError::InvalidPath::error: alloc::string::String pub aya::BpfError::MapError(aya::maps::MapError) pub aya::BpfError::NoBTF pub aya::BpfError::NoPinPath