Merge pull request #701 from nrxus/perf-event-owned-fd

aya: Return `OwnedFd` for `perf_event_open`.

aya/src/maps/perf/perf_buffer.rs

@@ -1,13 +1,13 @@
 use std::{
     ffi::c_void,
     io, mem,
-    os::fd::{AsRawFd, RawFd},
+    os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd},
     ptr, slice,
     sync::atomic::{self, AtomicPtr, Ordering},
 };
 
 use bytes::BytesMut;
-use libc::{c_int, close, munmap, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE};
+use libc::{c_int, munmap, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE};
 use thiserror::Error;
 
 use crate::{
@@ -15,7 +15,7 @@ use crate::{
         perf_event_header, perf_event_mmap_page,
         perf_event_type::{PERF_RECORD_LOST, PERF_RECORD_SAMPLE},
     },
-    sys::{perf_event_ioctl, perf_event_open_bpf},
+    sys::{perf_event_ioctl, perf_event_open_bpf, SysResult},
     PERF_EVENT_IOC_DISABLE, PERF_EVENT_IOC_ENABLE,
 };
 
@@ -88,7 +88,7 @@ pub(crate) struct PerfBuffer {
     buf: AtomicPtr<perf_event_mmap_page>,
     size: usize,
     page_size: usize,
-    fd: RawFd,
+    fd: OwnedFd,
 }
 
 impl PerfBuffer {
@@ -102,8 +102,7 @@ impl PerfBuffer {
         }
 
         let fd = perf_event_open_bpf(cpu_id as i32)
-            .map_err(|(_, io_error)| PerfBufferError::OpenError { io_error })?
-            as RawFd;
+            .map_err(|(_, io_error)| PerfBufferError::OpenError { io_error })?;
         let size = page_size * page_count;
         let buf = unsafe {
             mmap(
@@ -111,7 +110,7 @@ impl PerfBuffer {
                 size + page_size,
                 PROT_READ | PROT_WRITE,
                 MAP_SHARED,
-                fd,
+                fd.as_fd(),
                 0,
             )
         };
@@ -128,7 +127,7 @@ impl PerfBuffer {
             page_size,
         };
 
-        perf_event_ioctl(fd, PERF_EVENT_IOC_ENABLE, 0)
+        perf_event_ioctl(perf_buf.fd.as_fd(), PERF_EVENT_IOC_ENABLE, 0)
             .map_err(|(_, io_error)| PerfBufferError::PerfEventEnableError { io_error })?;
 
         Ok(perf_buf)
@@ -261,19 +260,24 @@ impl PerfBuffer {
 
 impl AsRawFd for PerfBuffer {
     fn as_raw_fd(&self) -> RawFd {
-        self.fd
+        self.fd.as_raw_fd()
+    }
+}
+
+impl AsFd for PerfBuffer {
+    fn as_fd(&self) -> BorrowedFd<'_> {
+        self.fd.as_fd()
     }
 }
 
 impl Drop for PerfBuffer {
     fn drop(&mut self) {
         unsafe {
-            let _ = perf_event_ioctl(self.fd, PERF_EVENT_IOC_DISABLE, 0);
+            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,
             );
-            close(self.fd);
         }
     }
 }
@@ -284,11 +288,11 @@ unsafe fn mmap(
     len: usize,
     prot: c_int,
     flags: c_int,
-    fd: i32,
+    fd: BorrowedFd<'_>,
     offset: libc::off_t,
 ) -> *mut c_void {
     #[cfg(not(test))]
-    return libc::mmap(addr, len, prot, flags, fd, offset);
+    return libc::mmap(addr, len, prot, flags, fd.as_raw_fd(), offset);
 
     #[cfg(test)]
     use crate::sys::TEST_MMAP_RET;

aya/src/programs/perf_attach.rs

@@ -1,11 +1,10 @@
 //! Perf attach links.
-use libc::close;
-use std::os::fd::RawFd;
+use std::os::fd::{AsFd as _, AsRawFd as _, OwnedFd, RawFd};
 
 use crate::{
     generated::bpf_attach_type::BPF_PERF_EVENT,
     programs::{probe::detach_debug_fs, FdLink, Link, ProbeKind, ProgramError},
-    sys::{bpf_link_create, perf_event_ioctl},
+    sys::{bpf_link_create, perf_event_ioctl, SysResult},
     FEATURES, PERF_EVENT_IOC_DISABLE, PERF_EVENT_IOC_ENABLE, PERF_EVENT_IOC_SET_BPF,
 };
 
@@ -46,7 +45,7 @@ pub struct PerfLinkId(RawFd);
 /// The attachment type of PerfEvent programs.
 #[derive(Debug)]
 pub struct PerfLink {
-    perf_fd: RawFd,
+    perf_fd: OwnedFd,
     probe_kind: Option<ProbeKind>,
     event_alias: Option<String>,
 }
@@ -55,16 +54,15 @@ impl Link for PerfLink {
     type Id = PerfLinkId;
 
     fn id(&self) -> Self::Id {
-        PerfLinkId(self.perf_fd)
+        PerfLinkId(self.perf_fd.as_raw_fd())
     }
 
     fn detach(mut self) -> Result<(), ProgramError> {
-        let _ = perf_event_ioctl(self.perf_fd, PERF_EVENT_IOC_DISABLE, 0);
-        unsafe { close(self.perf_fd) };
+        let _: SysResult<_> = perf_event_ioctl(self.perf_fd.as_fd(), PERF_EVENT_IOC_DISABLE, 0);
 
         if let Some(probe_kind) = self.probe_kind.take() {
             if let Some(event_alias) = self.event_alias.take() {
-                let _ = detach_debug_fs(probe_kind, &event_alias);
+                let _: Result<_, _> = detach_debug_fs(probe_kind, &event_alias);
             }
         }
 
@@ -72,15 +70,14 @@ impl Link for PerfLink {
     }
 }
 
-pub(crate) fn perf_attach(prog_fd: RawFd, fd: RawFd) -> Result<PerfLinkInner, ProgramError> {
+pub(crate) fn perf_attach(prog_fd: RawFd, fd: OwnedFd) -> Result<PerfLinkInner, ProgramError> {
     if FEATURES.bpf_perf_link() {
-        let link_fd =
-            bpf_link_create(prog_fd, fd, BPF_PERF_EVENT, None, 0).map_err(|(_, io_error)| {
-                ProgramError::SyscallError {
-                    call: "bpf_link_create",
-                    io_error,
-                }
-            })? as RawFd;
+        let link_fd = bpf_link_create(prog_fd, fd.as_raw_fd(), BPF_PERF_EVENT, None, 0).map_err(
+            |(_, io_error)| ProgramError::SyscallError {
+                call: "bpf_link_create",
+                io_error,
+            },
+        )? as RawFd;
         Ok(PerfLinkInner::FdLink(FdLink::new(link_fd)))
     } else {
         perf_attach_either(prog_fd, fd, None, None)
@@ -89,7 +86,7 @@ pub(crate) fn perf_attach(prog_fd: RawFd, fd: RawFd) -> Result<PerfLinkInner, Pr
 
 pub(crate) fn perf_attach_debugfs(
     prog_fd: RawFd,
-    fd: RawFd,
+    fd: OwnedFd,
     probe_kind: ProbeKind,
     event_alias: String,
 ) -> Result<PerfLinkInner, ProgramError> {
@@ -98,17 +95,17 @@ pub(crate) fn perf_attach_debugfs(
 
 fn perf_attach_either(
     prog_fd: RawFd,
-    fd: RawFd,
+    fd: OwnedFd,
     probe_kind: Option<ProbeKind>,
     event_alias: Option<String>,
 ) -> Result<PerfLinkInner, ProgramError> {
-    perf_event_ioctl(fd, PERF_EVENT_IOC_SET_BPF, prog_fd).map_err(|(_, io_error)| {
+    perf_event_ioctl(fd.as_fd(), PERF_EVENT_IOC_SET_BPF, prog_fd).map_err(|(_, io_error)| {
         ProgramError::SyscallError {
             call: "PERF_EVENT_IOC_SET_BPF",
             io_error,
         }
     })?;
-    perf_event_ioctl(fd, PERF_EVENT_IOC_ENABLE, 0).map_err(|(_, io_error)| {
+    perf_event_ioctl(fd.as_fd(), PERF_EVENT_IOC_ENABLE, 0).map_err(|(_, io_error)| {
         ProgramError::SyscallError {
             call: "PERF_EVENT_IOC_ENABLE",
             io_error,

aya/src/programs/perf_event.rs

@@ -168,7 +168,7 @@ impl PerfEvent {
         .map_err(|(_code, io_error)| ProgramError::SyscallError {
             call: "perf_event_open",
             io_error,
-        })? as i32;
+        })?;
 
         let link = perf_attach(self.data.fd_or_err()?, fd)?;
         self.data.links.insert(PerfEventLink::new(link))

aya/src/programs/probe.rs

@@ -3,6 +3,7 @@ use libc::pid_t;
 use std::{
     fs::{self, OpenOptions},
     io::{self, Write},
+    os::fd::OwnedFd,
     path::Path,
     process,
     sync::atomic::{AtomicUsize, Ordering},
@@ -86,7 +87,7 @@ fn create_as_probe(
     fn_name: &str,
     offset: u64,
     pid: Option<pid_t>,
-) -> Result<i32, ProgramError> {
+) -> Result<OwnedFd, ProgramError> {
     use ProbeKind::*;
 
     let perf_ty = match kind {
@@ -108,14 +109,12 @@ fn create_as_probe(
         _ => None,
     };
 
-    let fd = perf_event_open_probe(perf_ty, ret_bit, fn_name, offset, pid).map_err(
-        |(_code, io_error)| ProgramError::SyscallError {
+    perf_event_open_probe(perf_ty, ret_bit, fn_name, offset, pid).map_err(|(_code, io_error)| {
+        ProgramError::SyscallError {
             call: "perf_event_open",
             io_error,
-        },
-    )? as i32;
-
-    Ok(fd)
+        }
+    })
 }
 
 fn create_as_trace_point(
@@ -123,7 +122,7 @@ fn create_as_trace_point(
     name: &str,
     offset: u64,
     pid: Option<pid_t>,
-) -> Result<(i32, String), ProgramError> {
+) -> Result<(OwnedFd, String), ProgramError> {
     use ProbeKind::*;
 
     let tracefs = find_tracefs_path()?;
@@ -142,7 +141,7 @@ fn create_as_trace_point(
             call: "perf_event_open",
             io_error,
         }
-    })? as i32;
+    })?;
 
     Ok((fd, event_alias))
 }

aya/src/programs/trace_point.rs

@@ -82,10 +82,10 @@ impl TracePoint {
         let id = read_sys_fs_trace_point_id(tracefs, category, name)?;
         let fd = perf_event_open_trace_point(id, None).map_err(|(_code, io_error)| {
             ProgramError::SyscallError {
-                call: "perf_event_open",
+                call: "perf_event_open_trace_point",
                 io_error,
             }
-        })? as i32;
+        })?;
 
         let link = perf_attach(self.data.fd_or_err()?, fd)?;
         self.data.links.insert(TracePointLink::new(link))

aya/src/sys/mod.rs

@@ -5,7 +5,10 @@ mod perf_event;
 #[cfg(test)]
 mod fake;
 
-use std::{io, mem};
+use std::{
+    io, mem,
+    os::fd::{AsRawFd as _, BorrowedFd},
+};
 
 use libc::{c_int, c_long, pid_t, SYS_bpf, SYS_perf_event_open};
 
@@ -34,7 +37,7 @@ pub(crate) enum Syscall<'a> {
         flags: u32,
     },
     PerfEventIoctl {
-        fd: c_int,
+        fd: BorrowedFd<'a>,
         request: c_int,
         arg: c_int,
     },
@@ -90,7 +93,7 @@ fn syscall(call: Syscall) -> SysResult<c_long> {
                 flags,
             } => libc::syscall(SYS_perf_event_open, &attr, pid, cpu, group, flags),
             Syscall::PerfEventIoctl { fd, request, arg } => {
-                libc::ioctl(fd, request.try_into().unwrap(), arg) as libc::c_long
+                libc::ioctl(fd.as_raw_fd(), request.try_into().unwrap(), arg) as libc::c_long
             }
         }
     } {

aya/src/sys/perf_event.rs

@@ -1,6 +1,7 @@
 use std::{
     ffi::{c_long, CString},
-    mem,
+    io, mem,
+    os::fd::{BorrowedFd, FromRawFd as _, OwnedFd},
 };
 
 use libc::{c_int, pid_t};
@@ -25,7 +26,7 @@ pub(crate) fn perf_event_open(
     sample_frequency: Option<u64>,
     wakeup: bool,
     flags: u32,
-) -> SysResult<c_long> {
+) -> SysResult<OwnedFd> {
     let mut attr = unsafe { mem::zeroed::<perf_event_attr>() };
 
     attr.config = config;
@@ -42,16 +43,10 @@ pub(crate) fn perf_event_open(
         attr.__bindgen_anon_1.sample_period = sample_period;
     }
 
-    syscall(Syscall::PerfEventOpen {
-        attr,
-        pid,
-        cpu,
-        group: -1,
-        flags,
-    })
+    perf_event_sys(attr, pid, cpu, flags)
 }
 
-pub(crate) fn perf_event_open_bpf(cpu: c_int) -> SysResult<c_long> {
+pub(crate) fn perf_event_open_bpf(cpu: c_int) -> SysResult<OwnedFd> {
     perf_event_open(
         PERF_TYPE_SOFTWARE as u32,
         PERF_COUNT_SW_BPF_OUTPUT as u64,
@@ -70,7 +65,7 @@ pub(crate) fn perf_event_open_probe(
     name: &str,
     offset: u64,
     pid: Option<pid_t>,
-) -> SysResult<c_long> {
+) -> SysResult<OwnedFd> {
     let mut attr = unsafe { mem::zeroed::<perf_event_attr>() };
 
     if let Some(ret_bit) = ret_bit {
@@ -87,16 +82,10 @@ pub(crate) fn perf_event_open_probe(
     let cpu = if pid.is_some() { -1 } else { 0 };
     let pid = pid.unwrap_or(-1);
 
-    syscall(Syscall::PerfEventOpen {
-        attr,
-        pid,
-        cpu,
-        group: -1,
-        flags: PERF_FLAG_FD_CLOEXEC,
-    })
+    perf_event_sys(attr, pid, cpu, PERF_FLAG_FD_CLOEXEC)
 }
 
-pub(crate) fn perf_event_open_trace_point(id: u32, pid: Option<pid_t>) -> SysResult<c_long> {
+pub(crate) fn perf_event_open_trace_point(id: u32, pid: Option<pid_t>) -> SysResult<OwnedFd> {
     let mut attr = unsafe { mem::zeroed::<perf_event_attr>() };
 
     attr.size = mem::size_of::<perf_event_attr>() as u32;
@@ -106,16 +95,14 @@ pub(crate) fn perf_event_open_trace_point(id: u32, pid: Option<pid_t>) -> SysRes
     let cpu = if pid.is_some() { -1 } else { 0 };
     let pid = pid.unwrap_or(-1);
 
-    syscall(Syscall::PerfEventOpen {
-        attr,
-        pid,
-        cpu,
-        group: -1,
-        flags: PERF_FLAG_FD_CLOEXEC,
-    })
+    perf_event_sys(attr, pid, cpu, PERF_FLAG_FD_CLOEXEC)
 }
 
-pub(crate) fn perf_event_ioctl(fd: c_int, request: c_int, arg: c_int) -> SysResult<c_long> {
+pub(crate) fn perf_event_ioctl(
+    fd: BorrowedFd<'_>,
+    request: c_int,
+    arg: c_int,
+) -> SysResult<c_long> {
     let call = Syscall::PerfEventIoctl { fd, request, arg };
     #[cfg(not(test))]
     return syscall(call);
@@ -124,6 +111,29 @@ pub(crate) fn perf_event_ioctl(fd: c_int, request: c_int, arg: c_int) -> SysResu
     return crate::sys::TEST_SYSCALL.with(|test_impl| unsafe { test_impl.borrow()(call) });
 }
 
+fn perf_event_sys(attr: perf_event_attr, pid: pid_t, cpu: i32, flags: u32) -> SysResult<OwnedFd> {
+    let fd = syscall(Syscall::PerfEventOpen {
+        attr,
+        pid,
+        cpu,
+        group: -1,
+        flags,
+    })?;
+
+    let fd = fd.try_into().map_err(|_| {
+        (
+            fd,
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("perf_event_open: invalid fd returned: {fd}"),
+            ),
+        )
+    })?;
+
+    // SAFETY: perf_event_open returns a new file descriptor on success.
+    unsafe { Ok(OwnedFd::from_raw_fd(fd)) }
+}
+
 /*
 impl TryFrom<u32> for perf_event_type {
     PERF_RECORD_MMAP = 1,