diff --git a/aya/src/programs/cgroup_skb.rs b/aya/src/programs/cgroup_skb.rs index 63a8671d..28eb6fd5 100644 --- a/aya/src/programs/cgroup_skb.rs +++ b/aya/src/programs/cgroup_skb.rs @@ -93,7 +93,7 @@ impl CgroupSkb { io_error, } })? as RawFd; - Ok(FdLink { fd: Some(link_fd) }.into()) + Ok(FdLink { fd: link_fd }.into()) } else { bpf_prog_attach(prog_fd, cgroup_fd, attach_type).map_err(|(_, io_error)| { ProgramError::SyscallError { diff --git a/aya/src/programs/lirc_mode2.rs b/aya/src/programs/lirc_mode2.rs index e401aa09..d57c2b9f 100644 --- a/aya/src/programs/lirc_mode2.rs +++ b/aya/src/programs/lirc_mode2.rs @@ -1,13 +1,17 @@ -use std::os::unix::prelude::{AsRawFd, RawFd}; +use std::{ + mem::ManuallyDrop, + os::unix::prelude::{AsRawFd, RawFd}, +}; use crate::{ generated::{bpf_attach_type::BPF_LIRC_MODE2, bpf_prog_type::BPF_PROG_TYPE_LIRC_MODE2}, - programs::{load_program, query, Link, OwnedLink, ProgramData, ProgramError, ProgramInfo}, - sys::{bpf_obj_get_info_by_fd, bpf_prog_attach, bpf_prog_detach, bpf_prog_get_fd_by_id}, + programs::{ + load_program, query, InnerLink, Link, OwnedLink, ProgAttachLink, ProgramData, ProgramError, + ProgramInfo, + }, + sys::{bpf_obj_get_info_by_fd, bpf_prog_attach, bpf_prog_get_fd_by_id}, }; -use libc::{close, dup}; - /// A program used to decode IR into key events for a lirc device. /// /// [`LircMode2`] programs can be used to inspect infrared pulses, spaces, @@ -91,60 +95,61 @@ impl LircMode2 { Ok(prog_fds .into_iter() - .map(|prog_fd| LircLink { - prog_fd: Some(prog_fd), - target_fd: Some(unsafe { dup(target_fd.as_raw_fd()) }), - }) + .map(|prog_fd| LircLink::new(prog_fd, target_fd.as_raw_fd())) .collect()) } } #[derive(Debug)] pub struct LircLink { - prog_fd: Option, - target_fd: Option, + inner: ProgAttachLink, } impl LircLink { pub(crate) fn new(prog_fd: RawFd, target_fd: RawFd) -> LircLink { LircLink { - prog_fd: Some(prog_fd), - target_fd: Some(unsafe { dup(target_fd) }), + inner: ProgAttachLink::new(prog_fd, target_fd, BPF_LIRC_MODE2), } } pub fn info(&self) -> Result { - if let Some(fd) = self.prog_fd { - match bpf_obj_get_info_by_fd(fd) { - Ok(info) => Ok(ProgramInfo(info)), - Err(io_error) => Err(ProgramError::SyscallError { - call: "bpf_obj_get_info_by_fd".to_owned(), - io_error, - }), - } - } else { - Err(ProgramError::AlreadyDetached) - } + bpf_obj_get_info_by_fd(self.inner.prog_fd) + .map(ProgramInfo) + .map_err(|io_error| ProgramError::SyscallError { + call: "bpf_obj_get_info_by_fd".to_owned(), + io_error, + }) } } -impl Link for LircLink { +impl InnerLink for LircLink { fn detach(&mut self) -> Result<(), ProgramError> { - if let Some(prog_fd) = self.prog_fd.take() { - let target_fd = self.target_fd.take().unwrap(); - let _ = bpf_prog_detach(prog_fd, target_fd, BPF_LIRC_MODE2); - unsafe { close(target_fd) }; - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) - } + self.inner.detach() + } + + fn forget(&mut self) -> Result<(), ProgramError> { + self.inner.forget() } } +impl Link for LircLink { + fn detach(self) -> Result<(), ProgramError> { + let mut v = ManuallyDrop::new(self); + InnerLink::detach(&mut *v) + } + + fn forget(self) -> Result<(), ProgramError> { + let mut v = ManuallyDrop::new(self); + InnerLink::forget(&mut *v) + } +} + +// Since LircLinks can only be publicly created from query, they are essentially +// mutable views, and the actual ownership of the link lies with the +// kernel. Perhaps it is more appropriate to create a separate LircLinkView +// struct. impl Drop for LircLink { fn drop(&mut self) { - if let Some(target_fd) = self.target_fd.take() { - unsafe { close(target_fd) }; - } + let _ = self.forget(); } } diff --git a/aya/src/programs/lsm.rs b/aya/src/programs/lsm.rs index bc8646e6..80f5a80b 100644 --- a/aya/src/programs/lsm.rs +++ b/aya/src/programs/lsm.rs @@ -102,5 +102,5 @@ pub(crate) fn attach_btf_id(program_data: &mut ProgramData) -> Result( } } -/// Detach an attached program. +/// A type implementing Link represents an attached eBPF program. It can be +/// either detached to terminate its execution, or forgotten so that it persists +/// in the kernel even without a companion userspace. pub trait Link { + /// Detach the program from the eBPF VM. + fn detach(self) -> Result<(), ProgramError>; + /// Perform any necessary cleanup to forget the program without leaking + /// system resources, if possible. + fn forget(self) -> Result<(), ProgramError>; +} + +/// The private counterpart to Link for the enum members of OwnedLink. InnerLink functions are +/// permitted to put the implementing type in a state such that all subsequent method calls fail. +/// The intent is that InnerLink functions will only be publicly called through Link. This allows +/// us to cleanly handle Drop without exposing the &mut self methods to the public API. +pub(crate) trait InnerLink { + /// Detach the program from the eBPF VM. fn detach(&mut self) -> Result<(), ProgramError>; + /// Perform any necessary cleanup to forget the program without leaking + /// system resources, if possible. + fn forget(&mut self) -> Result<(), ProgramError> { + Ok(()) + } } /// The return type of `program.attach(...)`. /// -/// [`OwnedLink`] implements the [`Link`] trait and can be used to detach a -/// program. +/// [`OwnedLink`] implements the [`Link`] trait and can be used to detach or +/// forget a program. /// An eBPF program's lifetime is directly connected to the OwnedLink's; it must /// be in scope for as long as one wants the program to remain attached. When -/// dropped, OwnedLink will detach the program. +/// dropped, OwnedLink will detach the program. In order to persist a program in +/// the kernel beyond the OwnedLink's lifetime, call the [forget](Link::forget) method. #[derive(Debug)] pub struct OwnedLink { - pub(crate) inner: OwnedLinkImpl, + inner: OwnedLinkImpl, } impl Link for OwnedLink { - fn detach(&mut self) -> Result<(), ProgramError> { - self.inner.detach() + fn detach(self) -> Result<(), ProgramError> { + let mut v = ManuallyDrop::new(self); + v.inner.detach() + } + + fn forget(self) -> Result<(), ProgramError> { + let mut v = ManuallyDrop::new(self); + v.inner.forget() } } @@ -544,6 +564,12 @@ impl From for OwnedLink { } } +impl Drop for OwnedLink { + fn drop(&mut self) { + let _ = self.inner.detach(); + } +} + #[derive(Debug)] pub(crate) enum OwnedLinkImpl { Fd(FdLink), @@ -555,7 +581,7 @@ pub(crate) enum OwnedLinkImpl { Tc(TcLink), } -impl Link for OwnedLinkImpl { +impl OwnedLinkImpl { fn detach(&mut self) -> Result<(), ProgramError> { match self { Self::Fd(link) => link.detach(), @@ -567,6 +593,18 @@ impl Link for OwnedLinkImpl { Self::Tc(link) => link.detach(), } } + + fn forget(&mut self) -> Result<(), ProgramError> { + match self { + Self::Fd(link) => link.forget(), + Self::Lirc(link) => link.forget(), + Self::Nl(link) => link.forget(), + Self::Perf(link) => link.forget(), + Self::ProgAttach(link) => link.forget(), + Self::SocketFilter(link) => link.forget(), + Self::Tc(link) => link.forget(), + } + } } impl From for OwnedLinkImpl { @@ -613,30 +651,21 @@ impl From for OwnedLinkImpl { #[derive(Debug)] pub(crate) struct FdLink { - fd: Option, + fd: RawFd, } -impl Link for FdLink { +impl InnerLink for FdLink { fn detach(&mut self) -> Result<(), ProgramError> { - if let Some(fd) = self.fd.take() { - unsafe { close(fd) }; - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) - } - } -} - -impl Drop for FdLink { - fn drop(&mut self) { - let _ = self.detach(); + // TODO: Actually wrap this return code. + unsafe { close(self.fd) }; + Ok(()) } } #[derive(Debug)] pub(crate) struct ProgAttachLink { - prog_fd: Option, - target_fd: Option, + prog_fd: RawFd, + target_fd: RawFd, attach_type: bpf_attach_type, } @@ -647,29 +676,24 @@ impl ProgAttachLink { attach_type: bpf_attach_type, ) -> ProgAttachLink { ProgAttachLink { - prog_fd: Some(prog_fd), - target_fd: Some(unsafe { dup(target_fd) }), + prog_fd, + target_fd: unsafe { dup(target_fd) }, attach_type, } } } -impl Link for ProgAttachLink { +impl InnerLink for ProgAttachLink { fn detach(&mut self) -> Result<(), ProgramError> { - if let Some(prog_fd) = self.prog_fd.take() { - let target_fd = self.target_fd.take().unwrap(); - let _ = bpf_prog_detach(prog_fd, target_fd, self.attach_type); - unsafe { close(target_fd) }; - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) - } + // TODO: Actually wrap this return code. + let _ = bpf_prog_detach(self.prog_fd, self.target_fd, self.attach_type); + self.forget() } -} -impl Drop for ProgAttachLink { - fn drop(&mut self) { - let _ = self.detach(); + fn forget(&mut self) -> Result<(), ProgramError> { + // TODO: Actually wrap this return code. + unsafe { close(self.target_fd) }; + Ok(()) } } diff --git a/aya/src/programs/perf_attach.rs b/aya/src/programs/perf_attach.rs index 485f8a27..f7e6d5e2 100644 --- a/aya/src/programs/perf_attach.rs +++ b/aya/src/programs/perf_attach.rs @@ -7,42 +7,35 @@ use crate::{ PERF_EVENT_IOC_DISABLE, PERF_EVENT_IOC_ENABLE, PERF_EVENT_IOC_SET_BPF, }; -use super::{Link, OwnedLink, ProgramData, ProgramError}; +use super::{InnerLink, OwnedLink, ProgramData, ProgramError}; + +#[derive(Debug)] +struct PerfLinkInfo { + probe_kind: ProbeKind, + event_alias: String, +} #[derive(Debug)] pub(crate) struct PerfLink { - perf_fd: Option, - probe_kind: Option, - event_alias: Option, + perf_fd: RawFd, + info: Option, } -impl Link for PerfLink { +impl InnerLink for PerfLink { fn detach(&mut self) -> Result<(), ProgramError> { - if let Some(fd) = self.perf_fd.take() { - let _ = perf_event_ioctl(fd, PERF_EVENT_IOC_DISABLE, 0); - unsafe { close(fd) }; - - 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 _ = perf_event_ioctl(self.perf_fd, PERF_EVENT_IOC_DISABLE, 0); + unsafe { close(self.perf_fd) }; - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) + if let Some(info) = self.info.take() { + let _ = detach_debug_fs(info.probe_kind, &info.event_alias); } - } -} -impl Drop for PerfLink { - fn drop(&mut self) { - let _ = self.detach(); + Ok(()) } } pub(crate) fn perf_attach(data: &mut ProgramData, fd: RawFd) -> Result { - perf_attach_either(data, fd, None, None) + perf_attach_either(data, fd, None) } pub(crate) fn perf_attach_debugfs( @@ -51,14 +44,17 @@ pub(crate) fn perf_attach_debugfs( probe_kind: ProbeKind, event_alias: String, ) -> Result { - perf_attach_either(data, fd, Some(probe_kind), Some(event_alias)) + let info = PerfLinkInfo { + probe_kind, + event_alias, + }; + perf_attach_either(data, fd, Some(info)) } fn perf_attach_either( data: &mut ProgramData, fd: RawFd, - probe_kind: Option, - event_alias: Option, + info: Option, ) -> Result { let prog_fd = data.fd_or_err()?; perf_event_ioctl(fd, PERF_EVENT_IOC_SET_BPF, prog_fd).map_err(|(_, io_error)| { @@ -74,10 +70,5 @@ fn perf_attach_either( } })?; - Ok(PerfLink { - perf_fd: Some(fd), - probe_kind, - event_alias, - } - .into()) + Ok(PerfLink { perf_fd: fd, info }.into()) } diff --git a/aya/src/programs/raw_trace_point.rs b/aya/src/programs/raw_trace_point.rs index 6a43b364..9282610b 100644 --- a/aya/src/programs/raw_trace_point.rs +++ b/aya/src/programs/raw_trace_point.rs @@ -57,6 +57,6 @@ impl RawTracePoint { } })? as RawFd; - Ok(FdLink { fd: Some(pfd) }.into()) + Ok(FdLink { fd: pfd }.into()) } } diff --git a/aya/src/programs/socket_filter.rs b/aya/src/programs/socket_filter.rs index 249a8b34..87b87c68 100644 --- a/aya/src/programs/socket_filter.rs +++ b/aya/src/programs/socket_filter.rs @@ -7,7 +7,7 @@ use thiserror::Error; use crate::{ generated::{bpf_prog_type::BPF_PROG_TYPE_SOCKET_FILTER, SO_ATTACH_BPF, SO_DETACH_BPF}, - programs::{load_program, Link, OwnedLink, ProgramData, ProgramError}, + programs::{load_program, InnerLink, OwnedLink, ProgramData, ProgramError}, }; /// The type returned when attaching a [`SocketFilter`] fails. @@ -91,41 +91,27 @@ impl SocketFilter { .into()); } - Ok(SocketFilterLink { - socket, - prog_fd: Some(prog_fd), - } - .into()) + Ok(SocketFilterLink { socket, prog_fd }.into()) } } #[derive(Debug)] pub(crate) struct SocketFilterLink { socket: RawFd, - prog_fd: Option, + prog_fd: RawFd, } -impl Link for SocketFilterLink { +impl InnerLink for SocketFilterLink { fn detach(&mut self) -> Result<(), ProgramError> { - if let Some(fd) = self.prog_fd.take() { - unsafe { - setsockopt( - self.socket, - SOL_SOCKET, - SO_DETACH_BPF as i32, - &fd as *const _ as *const _, - mem::size_of::() as u32, - ); - } - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) + unsafe { + setsockopt( + self.socket, + SOL_SOCKET, + SO_DETACH_BPF as i32, + &self.prog_fd as *const _ as *const _, + mem::size_of::() as u32, + ); } - } -} - -impl Drop for SocketFilterLink { - fn drop(&mut self) { - let _ = self.detach(); + Ok(()) } } diff --git a/aya/src/programs/tc.rs b/aya/src/programs/tc.rs index 40e48ee4..252a77bc 100644 --- a/aya/src/programs/tc.rs +++ b/aya/src/programs/tc.rs @@ -4,14 +4,13 @@ use thiserror::Error; use std::{ ffi::{CStr, CString}, io, - os::unix::io::RawFd, }; use crate::{ generated::{ bpf_prog_type::BPF_PROG_TYPE_SCHED_CLS, TC_H_CLSACT, TC_H_MIN_EGRESS, TC_H_MIN_INGRESS, }, - programs::{load_program, Link, OwnedLink, ProgramData, ProgramError}, + programs::{load_program, InnerLink, OwnedLink, ProgramData, ProgramError}, sys::{ netlink_find_filter_with_name, netlink_qdisc_add_clsact, netlink_qdisc_attach, netlink_qdisc_detach, @@ -91,7 +90,6 @@ pub enum TcError { pub(crate) struct TcLink { if_index: i32, attach_type: TcAttachType, - prog_fd: Option, priority: u32, } @@ -136,28 +134,17 @@ impl SchedClassifier { Ok(TcLink { if_index: if_index as i32, attach_type, - prog_fd: Some(prog_fd), priority, } .into()) } } -impl Drop for TcLink { - fn drop(&mut self) { - let _ = self.detach(); - } -} - -impl Link for TcLink { +impl InnerLink for TcLink { fn detach(&mut self) -> Result<(), ProgramError> { - if self.prog_fd.take().is_some() { - unsafe { netlink_qdisc_detach(self.if_index, &self.attach_type, self.priority) } - .map_err(|io_error| TcError::NetlinkError { io_error })?; - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) - } + unsafe { netlink_qdisc_detach(self.if_index, &self.attach_type, self.priority) } + .map_err(|io_error| TcError::NetlinkError { io_error })?; + Ok(()) } } diff --git a/aya/src/programs/tp_btf.rs b/aya/src/programs/tp_btf.rs index 1f90351f..9708e577 100644 --- a/aya/src/programs/tp_btf.rs +++ b/aya/src/programs/tp_btf.rs @@ -93,6 +93,6 @@ impl BtfTracePoint { } })? as RawFd; - Ok(FdLink { fd: Some(pfd) }.into()) + Ok(FdLink { fd: pfd }.into()) } } diff --git a/aya/src/programs/xdp.rs b/aya/src/programs/xdp.rs index bc31431e..7f1a3756 100644 --- a/aya/src/programs/xdp.rs +++ b/aya/src/programs/xdp.rs @@ -8,7 +8,7 @@ use crate::{ bpf_attach_type::BPF_XDP, bpf_prog_type::BPF_PROG_TYPE_XDP, XDP_FLAGS_DRV_MODE, XDP_FLAGS_HW_MODE, XDP_FLAGS_REPLACE, XDP_FLAGS_SKB_MODE, XDP_FLAGS_UPDATE_IF_NOEXIST, }, - programs::{load_program, FdLink, Link, OwnedLink, ProgramData, ProgramError}, + programs::{load_program, FdLink, InnerLink, OwnedLink, ProgramData, ProgramError}, sys::{bpf_link_create, kernel_version, netlink_set_xdp_fd}, }; @@ -105,14 +105,14 @@ impl Xdp { io_error, }, )? as RawFd; - Ok(FdLink { fd: Some(link_fd) }.into()) + Ok(FdLink { fd: link_fd }.into()) } else { unsafe { netlink_set_xdp_fd(if_index, prog_fd, None, flags.bits) } .map_err(|io_error| XdpError::NetlinkError { io_error })?; Ok(NlLink { if_index, - prog_fd: Some(prog_fd), + prog_fd, flags, } .into()) @@ -123,29 +123,15 @@ impl Xdp { #[derive(Debug)] pub(crate) struct NlLink { if_index: i32, - prog_fd: Option, + prog_fd: RawFd, flags: XdpFlags, } -impl Link for NlLink { +impl InnerLink for NlLink { fn detach(&mut self) -> Result<(), ProgramError> { - if let Some(fd) = self.prog_fd.take() { - let k_ver = kernel_version().unwrap(); - let flags = if k_ver >= (5, 7, 0) { - self.flags.bits | XDP_FLAGS_REPLACE - } else { - self.flags.bits - }; - let _ = unsafe { netlink_set_xdp_fd(self.if_index, -1, Some(fd), flags) }; - Ok(()) - } else { - Err(ProgramError::AlreadyDetached) - } - } -} - -impl Drop for NlLink { - fn drop(&mut self) { - let _ = self.detach(); + let k_ver = kernel_version().unwrap(); + let old_fd = (k_ver >= (5, 7, 0)).then(|| self.prog_fd); + let _ = unsafe { netlink_set_xdp_fd(self.if_index, -1, old_fd, self.flags.bits) }; + Ok(()) } } diff --git a/aya/src/sys/netlink.rs b/aya/src/sys/netlink.rs index f9e4af40..082683db 100644 --- a/aya/src/sys/netlink.rs +++ b/aya/src/sys/netlink.rs @@ -27,7 +27,7 @@ pub(crate) unsafe fn netlink_set_xdp_fd( if_index: i32, fd: RawFd, old_fd: Option, - flags: u32, + mut flags: u32, ) -> Result<(), io::Error> { let sock = NetlinkSocket::open()?; @@ -50,12 +50,12 @@ pub(crate) unsafe fn netlink_set_xdp_fd( let mut attrs = NestedAttrs::new(attrs_buf, IFLA_XDP); attrs.write_attr(IFLA_XDP_FD as u16, fd)?; - if flags > 0 { - attrs.write_attr(IFLA_XDP_FLAGS as u16, flags)?; + if let Some(old_fd) = old_fd { + attrs.write_attr(IFLA_XDP_EXPECTED_FD as u16, old_fd)?; + flags |= XDP_FLAGS_REPLACE; } - - if flags & XDP_FLAGS_REPLACE != 0 { - attrs.write_attr(IFLA_XDP_EXPECTED_FD as u16, old_fd.unwrap())?; + if flags != 0 { + attrs.write_attr(IFLA_XDP_FLAGS as u16, flags)?; } let nla_len = attrs.finish()?;