From 055e36e8d92c79c1a9cf61f1a08d2d9be4e74d14 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sat, 1 Mar 2025 08:54:28 -0500 Subject: [PATCH] aya: encode perf_event_open(2) contract Per man 2 perf_event_open: > RETURN VALUE > On success, perf_event_open() returns the new file descriptor. On > error, -1 is returned and errno is set to indicate the error. Bake this into our syscalls so we stop using `_` so much which can hide information loss. Remove the type parameter to SysResult. --- aya/src/maps/bloom_filter.rs | 4 ++-- aya/src/maps/hash_map/hash_map.rs | 12 +++++----- aya/src/maps/hash_map/per_cpu_hash_map.rs | 4 ++-- aya/src/maps/lpm_trie.rs | 4 ++-- aya/src/maps/perf/perf_buffer.rs | 4 ++-- aya/src/programs/perf_attach.rs | 2 +- aya/src/programs/perf_event.rs | 2 +- aya/src/programs/probe.rs | 4 ++-- aya/src/programs/trace_point.rs | 9 ++++---- aya/src/sys/fake.rs | 12 ++++------ aya/src/sys/mod.rs | 4 ++-- aya/src/sys/perf_event.rs | 27 ++++++++++++----------- 12 files changed, 42 insertions(+), 46 deletions(-) diff --git a/aya/src/maps/bloom_filter.rs b/aya/src/maps/bloom_filter.rs index c7c4d2fd..1d8b2c54 100644 --- a/aya/src/maps/bloom_filter.rs +++ b/aya/src/maps/bloom_filter.rs @@ -83,7 +83,7 @@ impl, V: Pod> BloomFilter { #[cfg(test)] mod tests { - use std::{ffi::c_long, io}; + use std::io; use assert_matches::assert_matches; use aya_obj::generated::{ @@ -105,7 +105,7 @@ mod tests { test_utils::new_obj_map::(BPF_MAP_TYPE_BLOOM_FILTER) } - fn sys_error(value: i32) -> SysResult { + fn sys_error(value: i32) -> SysResult { Err((-1, io::Error::from_raw_os_error(value))) } diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs index 1600a7d5..70c54c56 100644 --- a/aya/src/maps/hash_map/hash_map.rs +++ b/aya/src/maps/hash_map/hash_map.rs @@ -103,7 +103,7 @@ impl, K: Pod, V: Pod> IterableMap for HashMap #[cfg(test)] mod tests { - use std::{ffi::c_long, io}; + use std::io; use assert_matches::assert_matches; use aya_obj::generated::{ @@ -125,7 +125,7 @@ mod tests { test_utils::new_obj_map::(BPF_MAP_TYPE_HASH) } - fn sys_error(value: i32) -> SysResult { + fn sys_error(value: i32) -> SysResult { Err((-1, io::Error::from_raw_os_error(value))) } @@ -306,13 +306,13 @@ mod tests { } } - fn set_next_key(attr: &bpf_attr, next: T) -> SysResult { + fn set_next_key(attr: &bpf_attr, next: T) -> SysResult { let key = unsafe { attr.__bindgen_anon_2.__bindgen_anon_1.next_key } as *const T as *mut T; unsafe { *key = next }; Ok(0) } - fn set_ret(attr: &bpf_attr, ret: T) -> SysResult { + fn set_ret(attr: &bpf_attr, ret: T) -> SysResult { let value = unsafe { attr.__bindgen_anon_2.__bindgen_anon_1.value } as *const T as *mut T; unsafe { *value = ret }; Ok(0) @@ -333,7 +333,7 @@ mod tests { assert_matches!(keys, Ok(ks) if ks.is_empty()) } - fn get_next_key(attr: &bpf_attr) -> SysResult { + fn get_next_key(attr: &bpf_attr) -> SysResult { match bpf_key(attr) { None => set_next_key(attr, 10), Some(10) => set_next_key(attr, 20), @@ -343,7 +343,7 @@ mod tests { } } - fn lookup_elem(attr: &bpf_attr) -> SysResult { + fn lookup_elem(attr: &bpf_attr) -> SysResult { match bpf_key(attr) { Some(10) => set_ret(attr, 100), Some(20) => set_ret(attr, 200), diff --git a/aya/src/maps/hash_map/per_cpu_hash_map.rs b/aya/src/maps/hash_map/per_cpu_hash_map.rs index 5a81ff93..eed81b71 100644 --- a/aya/src/maps/hash_map/per_cpu_hash_map.rs +++ b/aya/src/maps/hash_map/per_cpu_hash_map.rs @@ -149,7 +149,7 @@ impl, K: Pod, V: Pod> IterableMap> #[cfg(test)] mod tests { - use std::{ffi::c_long, io}; + use std::io; use assert_matches::assert_matches; use aya_obj::generated::bpf_map_type::{ @@ -163,7 +163,7 @@ mod tests { sys::{override_syscall, SysResult}, }; - fn sys_error(value: i32) -> SysResult { + fn sys_error(value: i32) -> SysResult { Err((-1, io::Error::from_raw_os_error(value))) } diff --git a/aya/src/maps/lpm_trie.rs b/aya/src/maps/lpm_trie.rs index e4840533..895fb24d 100644 --- a/aya/src/maps/lpm_trie.rs +++ b/aya/src/maps/lpm_trie.rs @@ -190,7 +190,7 @@ impl, K: Pod, V: Pod> IterableMap, V> for LpmTrie>(BPF_MAP_TYPE_LPM_TRIE) } - fn sys_error(value: i32) -> SysResult { + fn sys_error(value: i32) -> SysResult { Err((-1, io::Error::from_raw_os_error(value))) } diff --git a/aya/src/maps/perf/perf_buffer.rs b/aya/src/maps/perf/perf_buffer.rs index 5b0c83bd..2a7989e7 100644 --- a/aya/src/maps/perf/perf_buffer.rs +++ b/aya/src/maps/perf/perf_buffer.rs @@ -102,7 +102,7 @@ impl PerfBuffer { } let fd = perf_event_open_bpf(cpu_id as i32) - .map_err(|(_, io_error)| PerfBufferError::OpenError { io_error })?; + .map_err(|io_error| PerfBufferError::OpenError { io_error })?; let size = page_size * page_count; let mmap = MMap::new( fd.as_fd(), @@ -260,7 +260,7 @@ impl AsFd for PerfBuffer { impl Drop for PerfBuffer { fn drop(&mut self) { - let _: SysResult<_> = perf_event_ioctl(self.fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0); + let _: SysResult = perf_event_ioctl(self.fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0); } } diff --git a/aya/src/programs/perf_attach.rs b/aya/src/programs/perf_attach.rs index 15762016..2b8fbfcc 100644 --- a/aya/src/programs/perf_attach.rs +++ b/aya/src/programs/perf_attach.rs @@ -71,7 +71,7 @@ impl Link for PerfLink { fn detach(self) -> Result<(), ProgramError> { let Self { perf_fd, event } = self; - let _: SysResult<_> = perf_event_ioctl(perf_fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0); + let _: SysResult = perf_event_ioctl(perf_fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0); if let Some(event) = event { let _: Result<_, _> = detach_debug_fs(event); } diff --git a/aya/src/programs/perf_event.rs b/aya/src/programs/perf_event.rs index 1ac07496..9dad3591 100644 --- a/aya/src/programs/perf_event.rs +++ b/aya/src/programs/perf_event.rs @@ -176,7 +176,7 @@ impl PerfEvent { inherit, 0, ) - .map_err(|(_code, io_error)| SyscallError { + .map_err(|io_error| SyscallError { call: "perf_event_open", io_error, })?; diff --git a/aya/src/programs/probe.rs b/aya/src/programs/probe.rs index aec7fae3..d197667d 100644 --- a/aya/src/programs/probe.rs +++ b/aya/src/programs/probe.rs @@ -177,7 +177,7 @@ fn create_as_probe( }; perf_event_open_probe(perf_ty, ret_bit, fn_name, offset, pid) - .map_err(|(_code, io_error)| SyscallError { + .map_err(|io_error| SyscallError { call: "perf_event_open", io_error, }) @@ -203,7 +203,7 @@ fn create_as_trace_point( let category = format!("{}s", kind.pmu()); 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 { + let fd = perf_event_open_trace_point(tpid, pid).map_err(|io_error| SyscallError { call: "perf_event_open", io_error, })?; diff --git a/aya/src/programs/trace_point.rs b/aya/src/programs/trace_point.rs index 45ff5e55..91dc51cb 100644 --- a/aya/src/programs/trace_point.rs +++ b/aya/src/programs/trace_point.rs @@ -76,11 +76,10 @@ impl TracePoint { let prog_fd = prog_fd.as_fd(); let tracefs = find_tracefs_path()?; 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", - io_error, - })?; + let fd = perf_event_open_trace_point(id, None).map_err(|io_error| SyscallError { + call: "perf_event_open_trace_point", + io_error, + })?; let link = perf_attach(prog_fd, fd, None /* cookie */)?; self.data.links.insert(TracePointLink::new(link)) diff --git a/aya/src/sys/fake.rs b/aya/src/sys/fake.rs index 47019926..a994cbfa 100644 --- a/aya/src/sys/fake.rs +++ b/aya/src/sys/fake.rs @@ -1,12 +1,8 @@ -use std::{ - cell::RefCell, - ffi::{c_long, c_void}, - io, ptr, -}; +use std::{cell::RefCell, ffi::c_void, io, ptr}; use super::{SysResult, Syscall}; -type SyscallFn = unsafe fn(Syscall<'_>) -> SysResult; +type SyscallFn = unsafe fn(Syscall<'_>) -> SysResult; #[cfg(test)] thread_local! { @@ -15,11 +11,11 @@ thread_local! { } #[cfg(test)] -unsafe fn test_syscall(_call: Syscall<'_>) -> SysResult { +unsafe fn test_syscall(_call: Syscall<'_>) -> SysResult { Err((-1, io::Error::from_raw_os_error(libc::EINVAL))) } #[cfg(test)] -pub(crate) fn override_syscall(call: unsafe fn(Syscall<'_>) -> SysResult) { +pub(crate) fn override_syscall(call: unsafe fn(Syscall<'_>) -> SysResult) { TEST_SYSCALL.with(|test_impl| *test_impl.borrow_mut() = call); } diff --git a/aya/src/sys/mod.rs b/aya/src/sys/mod.rs index 9734f2bb..04ec669a 100644 --- a/aya/src/sys/mod.rs +++ b/aya/src/sys/mod.rs @@ -24,7 +24,7 @@ pub(crate) use netlink::*; pub(crate) use perf_event::*; use thiserror::Error; -pub(crate) type SysResult = Result; +pub(crate) type SysResult = Result; pub(crate) enum Syscall<'a> { Ebpf { @@ -88,7 +88,7 @@ impl std::fmt::Debug for Syscall<'_> { } } -fn syscall(call: Syscall<'_>) -> SysResult { +fn syscall(call: Syscall<'_>) -> SysResult { #[cfg(test)] return TEST_SYSCALL.with(|test_impl| unsafe { test_impl.borrow()(call) }); diff --git a/aya/src/sys/perf_event.rs b/aya/src/sys/perf_event.rs index 972d922e..d0f161ed 100644 --- a/aya/src/sys/perf_event.rs +++ b/aya/src/sys/perf_event.rs @@ -1,5 +1,5 @@ use std::{ - ffi::{c_int, c_long, CString, OsStr}, + ffi::{c_int, CString, OsStr}, io, mem, os::fd::{BorrowedFd, FromRawFd as _}, }; @@ -26,7 +26,7 @@ pub(crate) fn perf_event_open( wakeup: bool, inherit: bool, flags: u32, -) -> SysResult { +) -> io::Result { let mut attr = unsafe { mem::zeroed::() }; attr.config = config; @@ -46,7 +46,7 @@ pub(crate) fn perf_event_open( perf_event_sys(attr, pid, cpu, flags) } -pub(crate) fn perf_event_open_bpf(cpu: c_int) -> SysResult { +pub(crate) fn perf_event_open_bpf(cpu: c_int) -> io::Result { perf_event_open( PERF_TYPE_SOFTWARE as u32, PERF_COUNT_SW_BPF_OUTPUT as u64, @@ -66,7 +66,7 @@ pub(crate) fn perf_event_open_probe( name: &OsStr, offset: u64, pid: Option, -) -> SysResult { +) -> io::Result { use std::os::unix::ffi::OsStrExt as _; let mut attr = unsafe { mem::zeroed::() }; @@ -91,7 +91,7 @@ pub(crate) fn perf_event_open_probe( pub(crate) fn perf_event_open_trace_point( id: u32, pid: Option, -) -> SysResult { +) -> io::Result { let mut attr = unsafe { mem::zeroed::() }; attr.size = mem::size_of::() as u32; @@ -104,7 +104,7 @@ pub(crate) fn perf_event_open_trace_point( perf_event_sys(attr, pid, cpu, PERF_FLAG_FD_CLOEXEC) } -pub(crate) fn perf_event_ioctl(fd: BorrowedFd<'_>, request: u32, arg: c_int) -> SysResult { +pub(crate) fn perf_event_ioctl(fd: BorrowedFd<'_>, request: u32, arg: c_int) -> SysResult { let call = Syscall::PerfEventIoctl { fd, request, arg }; #[cfg(not(test))] return syscall(call); @@ -118,22 +118,23 @@ fn perf_event_sys( pid: pid_t, cpu: i32, flags: u32, -) -> SysResult { +) -> io::Result { let fd = syscall(Syscall::PerfEventOpen { attr, pid, cpu, group: -1, flags, + }) + .map_err(|(code, io_error)| { + assert_eq!(code, -1); + io_error })?; let fd = fd.try_into().map_err(|std::num::TryFromIntError { .. }| { - ( - fd, - io::Error::new( - io::ErrorKind::InvalidData, - format!("perf_event_open: invalid fd returned: {fd}"), - ), + io::Error::new( + io::ErrorKind::InvalidData, + format!("perf_event_open: invalid fd returned: {fd}"), ) })?;