diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index c7bf0da1..c20a01bb 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -60,7 +60,9 @@ use std::{ }; use aya_obj::{generated::bpf_map_type, parse_map_info, EbpfSectionKind, InvalidTypeBinding}; -use libc::{getrlimit, rlim_t, rlimit, RLIMIT_MEMLOCK, RLIM_INFINITY}; +use libc::{ + c_int, c_void, getrlimit, off_t, rlim_t, rlimit, MAP_FAILED, RLIMIT_MEMLOCK, RLIM_INFINITY, +}; use log::warn; use thiserror::Error; @@ -68,7 +70,7 @@ use crate::{ pin::PinError, sys::{ bpf_create_map, bpf_get_object, bpf_map_freeze, bpf_map_get_fd_by_id, bpf_map_get_next_key, - bpf_map_update_elem_ptr, bpf_pin_object, SyscallError, + bpf_map_update_elem_ptr, bpf_pin_object, mmap, munmap, SyscallError, }, util::{nr_cpus, KernelVersion}, PinningType, Pod, @@ -952,6 +954,62 @@ impl Deref for PerCpuValues { } } +// MMap corresponds to a memory-mapped region. +// +// The data is unmapped in Drop. +#[cfg_attr(test, derive(Debug))] +struct MMap { + ptr: ptr::NonNull, + len: usize, +} + +// Needed because NonNull is !Send and !Sync out of caution that the data +// might be aliased unsafely. +unsafe impl Send for MMap {} +unsafe impl Sync for MMap {} + +impl MMap { + fn new( + fd: BorrowedFd<'_>, + len: usize, + prot: c_int, + flags: c_int, + offset: off_t, + ) -> Result { + match unsafe { mmap(ptr::null_mut(), len, prot, flags, fd, offset) } { + MAP_FAILED => Err(SyscallError { + call: "mmap", + io_error: io::Error::last_os_error(), + }), + ptr => { + let ptr = ptr::NonNull::new(ptr).ok_or( + // This should never happen, but to be paranoid, and so we never need to talk + // about a null pointer, we check it anyway. + SyscallError { + call: "mmap", + io_error: io::Error::other("mmap returned null pointer"), + }, + )?; + Ok(Self { ptr, len }) + } + } + } +} + +impl AsRef<[u8]> for MMap { + fn as_ref(&self) -> &[u8] { + let Self { ptr, len } = self; + unsafe { std::slice::from_raw_parts(ptr.as_ptr().cast(), *len) } + } +} + +impl Drop for MMap { + fn drop(&mut self) { + let Self { ptr, len } = *self; + unsafe { munmap(ptr.as_ptr(), len) }; + } +} + #[cfg(test)] mod test_utils { use aya_obj::{ diff --git a/aya/src/maps/perf/perf_buffer.rs b/aya/src/maps/perf/perf_buffer.rs index aad7d448..e04e1e2c 100644 --- a/aya/src/maps/perf/perf_buffer.rs +++ b/aya/src/maps/perf/perf_buffer.rs @@ -1,9 +1,8 @@ use std::{ - ffi::c_void, io, mem, os::fd::{AsFd, BorrowedFd}, ptr, slice, - sync::atomic::{self, AtomicPtr, Ordering}, + sync::atomic::{self, Ordering}, }; use aya_obj::generated::{ @@ -12,10 +11,13 @@ use aya_obj::generated::{ PERF_EVENT_IOC_DISABLE, PERF_EVENT_IOC_ENABLE, }; use bytes::BytesMut; -use libc::{munmap, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE}; +use libc::{MAP_SHARED, PROT_READ, PROT_WRITE}; use thiserror::Error; -use crate::sys::{mmap, perf_event_ioctl, perf_event_open_bpf, SysResult}; +use crate::{ + maps::MMap, + sys::{perf_event_ioctl, perf_event_open_bpf, SysResult, SyscallError}, +}; /// Perf buffer error. #[derive(Error, Debug)] @@ -81,9 +83,9 @@ pub struct Events { pub lost: usize, } -#[derive(Debug)] +#[cfg_attr(test, derive(Debug))] pub(crate) struct PerfBuffer { - buf: AtomicPtr, + mmap: MMap, size: usize, page_size: usize, fd: crate::MockableFd, @@ -102,24 +104,17 @@ impl PerfBuffer { let fd = perf_event_open_bpf(cpu_id as i32) .map_err(|(_, io_error)| PerfBufferError::OpenError { io_error })?; let size = page_size * page_count; - let buf = unsafe { - mmap( - ptr::null_mut(), - size + page_size, - PROT_READ | PROT_WRITE, - MAP_SHARED, - fd.as_fd(), - 0, - ) - }; - if buf == MAP_FAILED { - return Err(PerfBufferError::MMapError { - io_error: io::Error::last_os_error(), - }); - } + let mmap = MMap::new( + fd.as_fd(), + size + page_size, + PROT_READ | PROT_WRITE, + MAP_SHARED, + 0, + ) + .map_err(|SyscallError { call: _, io_error }| PerfBufferError::MMapError { io_error })?; let perf_buf = Self { - buf: AtomicPtr::new(buf as *mut perf_event_mmap_page), + mmap, size, page_size, fd, @@ -131,8 +126,12 @@ impl PerfBuffer { Ok(perf_buf) } + fn buf(&self) -> ptr::NonNull { + self.mmap.ptr.cast() + } + pub(crate) fn readable(&self) -> bool { - let header = self.buf.load(Ordering::SeqCst); + let header = self.buf().as_ptr(); let head = unsafe { (*header).data_head } as usize; let tail = unsafe { (*header).data_tail } as usize; head != tail @@ -145,7 +144,7 @@ impl PerfBuffer { if buffers.is_empty() { return Err(PerfBufferError::NoBuffers); } - let header = self.buf.load(Ordering::SeqCst); + let header = self.buf().as_ptr(); let base = header as usize + self.page_size; let mut events = Events { read: 0, lost: 0 }; @@ -265,13 +264,7 @@ impl AsFd for PerfBuffer { impl Drop for PerfBuffer { fn drop(&mut self) { - unsafe { - let _: SysResult<_> = perf_event_ioctl(self.fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0); - munmap( - self.buf.load(Ordering::SeqCst) as *mut c_void, - self.size + self.page_size, - ); - } + let _: SysResult<_> = perf_event_ioctl(self.fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0); } } diff --git a/aya/src/maps/ring_buf.rs b/aya/src/maps/ring_buf.rs index 65125464..7e31e34f 100644 --- a/aya/src/maps/ring_buf.rs +++ b/aya/src/maps/ring_buf.rs @@ -6,23 +6,18 @@ use std::{ borrow::Borrow, - ffi::{c_int, c_void}, fmt::{self, Debug, Formatter}, - io, mem, + mem, ops::Deref, os::fd::{AsFd as _, AsRawFd, BorrowedFd, RawFd}, - ptr, - ptr::NonNull, - slice, sync::atomic::{AtomicU32, AtomicUsize, Ordering}, }; use aya_obj::generated::{BPF_RINGBUF_BUSY_BIT, BPF_RINGBUF_DISCARD_BIT, BPF_RINGBUF_HDR_SZ}; -use libc::{munmap, off_t, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE}; +use libc::{MAP_SHARED, PROT_READ, PROT_WRITE}; use crate::{ - maps::{MapData, MapError}, - sys::{mmap, SyscallError}, + maps::{MMap, MapData, MapError}, util::page_size, }; @@ -403,58 +398,3 @@ impl ProducerData { } } } - -// MMap corresponds to a memory-mapped region. -// -// The data is unmapped in Drop. -struct MMap { - ptr: NonNull, - len: usize, -} - -// Needed because NonNull is !Send and !Sync out of caution that the data -// might be aliased unsafely. -unsafe impl Send for MMap {} -unsafe impl Sync for MMap {} - -impl MMap { - fn new( - fd: BorrowedFd<'_>, - len: usize, - prot: c_int, - flags: c_int, - offset: off_t, - ) -> Result { - match unsafe { mmap(ptr::null_mut(), len, prot, flags, fd, offset) } { - MAP_FAILED => Err(MapError::SyscallError(SyscallError { - call: "mmap", - io_error: io::Error::last_os_error(), - })), - ptr => Ok(Self { - ptr: NonNull::new(ptr).ok_or( - // This should never happen, but to be paranoid, and so we never need to talk - // about a null pointer, we check it anyway. - MapError::SyscallError(SyscallError { - call: "mmap", - io_error: io::Error::other("mmap returned null pointer"), - }), - )?, - len, - }), - } - } -} - -impl AsRef<[u8]> for MMap { - fn as_ref(&self) -> &[u8] { - let Self { ptr, len } = self; - unsafe { slice::from_raw_parts(ptr.as_ptr().cast(), *len) } - } -} - -impl Drop for MMap { - fn drop(&mut self) { - let Self { ptr, len } = *self; - unsafe { munmap(ptr.as_ptr(), len) }; - } -} diff --git a/aya/src/sys/mod.rs b/aya/src/sys/mod.rs index 8eab8acd..9734f2bb 100644 --- a/aya/src/sys/mod.rs +++ b/aya/src/sys/mod.rs @@ -135,6 +135,15 @@ pub(crate) unsafe fn mmap( TEST_MMAP_RET.with(|ret| *ret.borrow()) } +#[cfg_attr(test, allow(unused_variables))] +pub(crate) unsafe fn munmap(addr: *mut c_void, len: usize) -> c_int { + #[cfg(not(test))] + return libc::munmap(addr, len); + + #[cfg(test)] + 0 +} + /// The type of eBPF statistic to enable. #[non_exhaustive] #[doc(alias = "bpf_stats_type")] diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index 158e82d5..1c26bbdd 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -434,7 +434,7 @@ pub fn aya::maps::perf::AsyncPerfEventArray::from(t: T) -> T pub struct aya::maps::perf::AsyncPerfEventArrayBuffer> impl> aya::maps::perf::AsyncPerfEventArrayBuffer pub async fn aya::maps::perf::AsyncPerfEventArrayBuffer::read_events(&mut self, buffers: &mut [bytes::bytes_mut::BytesMut]) -> core::result::Result -impl !core::marker::Freeze for aya::maps::perf::AsyncPerfEventArrayBuffer +impl core::marker::Freeze for aya::maps::perf::AsyncPerfEventArrayBuffer impl core::marker::Send for aya::maps::perf::AsyncPerfEventArrayBuffer where T: core::marker::Sync + core::marker::Send impl core::marker::Sync for aya::maps::perf::AsyncPerfEventArrayBuffer where T: core::marker::Sync + core::marker::Send impl core::marker::Unpin for aya::maps::perf::AsyncPerfEventArrayBuffer @@ -533,7 +533,7 @@ impl> std::os::fd::owned::AsFd fo pub fn aya::maps::perf::PerfEventArrayBuffer::as_fd(&self) -> std::os::fd::owned::BorrowedFd<'_> impl> std::os::fd::raw::AsRawFd for aya::maps::perf::PerfEventArrayBuffer pub fn aya::maps::perf::PerfEventArrayBuffer::as_raw_fd(&self) -> std::os::fd::raw::RawFd -impl !core::marker::Freeze for aya::maps::perf::PerfEventArrayBuffer +impl core::marker::Freeze for aya::maps::perf::PerfEventArrayBuffer impl core::marker::Send for aya::maps::perf::PerfEventArrayBuffer where T: core::marker::Sync + core::marker::Send impl core::marker::Sync for aya::maps::perf::PerfEventArrayBuffer where T: core::marker::Sync + core::marker::Send impl core::marker::Unpin for aya::maps::perf::PerfEventArrayBuffer