aya: modify Link trait to take ownership in detach and forget.

This fixes public LircLink drop handling and cleans up the unnecessary
Option wrappers in FdLink and ProgAttachLink.

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 {

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<RawFd>,
-    target_fd: Option<RawFd>,
+    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<ProgramInfo, ProgramError> {
-        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 {
+        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,
-                }),
+            })
     }
-        } else {
-            Err(ProgramError::AlreadyDetached)
+}
+
+impl InnerLink for LircLink {
+    fn detach(&mut self) -> Result<(), ProgramError> {
+        self.inner.detach()
     }
+
+    fn forget(&mut self) -> Result<(), ProgramError> {
+        self.inner.forget()
     }
 }
 
 impl Link 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)
+    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();
     }
 }

aya/src/programs/lsm.rs

@@ -102,5 +102,5 @@ pub(crate) fn attach_btf_id(program_data: &mut ProgramData) -> Result<OwnedLink,
         }
     })? as RawFd;
 
-    Ok(FdLink { fd: Some(pfd) }.into())
+    Ok(FdLink { fd: pfd }.into())
 }

aya/src/programs/mod.rs

@@ -60,6 +60,7 @@ use std::{
     convert::TryFrom,
     ffi::{CStr, CString},
     io,
+    mem::ManuallyDrop,
     os::unix::io::{AsRawFd, RawFd},
     path::Path,
 };
@@ -105,14 +106,6 @@ pub enum ProgramError {
     #[error("the program is not loaded")]
     NotLoaded,
 
-    /// The program is already detached.
-    #[error("the program was already detached")]
-    AlreadyDetached,
-
-    /// The program is not attached.
-    #[error("the program is not attached")]
-    NotAttached,
-
     /// Loading the program failed.
     #[error("the BPF_PROG_LOAD syscall failed. Verifier output: {verifier_log}")]
     LoadError {
@@ -473,26 +466,53 @@ pub(crate) fn query<T: AsRawFd>(
     }
 }
 
-/// 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<TcLink> 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<FdLink> for OwnedLinkImpl {
@@ -613,30 +651,21 @@ impl From<TcLink> for OwnedLinkImpl {
 
 #[derive(Debug)]
 pub(crate) struct FdLink {
-    fd: Option<RawFd>,
+    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) };
+        // TODO: Actually wrap this return code.
+        unsafe { close(self.fd) };
         Ok(())
-        } else {
-            Err(ProgramError::AlreadyDetached)
-        }
-    }
-}
-
-impl Drop for FdLink {
-    fn drop(&mut self) {
-        let _ = self.detach();
     }
 }
 
 #[derive(Debug)]
 pub(crate) struct ProgAttachLink {
-    prog_fd: Option<RawFd>,
-    target_fd: Option<RawFd>,
+    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(())
     }
 }
 

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<RawFd>,
-    probe_kind: Option<ProbeKind>,
-    event_alias: Option<String>,
+    perf_fd: RawFd,
+    info: Option<PerfLinkInfo>,
 }
 
-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) };
+        let _ = perf_event_ioctl(self.perf_fd, PERF_EVENT_IOC_DISABLE, 0);
+        unsafe { close(self.perf_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);
-                }
+        if let Some(info) = self.info.take() {
+            let _ = detach_debug_fs(info.probe_kind, &info.event_alias);
         }
 
         Ok(())
-        } else {
-            Err(ProgramError::AlreadyDetached)
-        }
-    }
-}
-
-impl Drop for PerfLink {
-    fn drop(&mut self) {
-        let _ = self.detach();
     }
 }
 
 pub(crate) fn perf_attach(data: &mut ProgramData, fd: RawFd) -> Result<OwnedLink, ProgramError> {
-    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<OwnedLink, ProgramError> {
-    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<ProbeKind>,
-    event_alias: Option<String>,
+    info: Option<PerfLinkInfo>,
 ) -> Result<OwnedLink, ProgramError> {
     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())
 }

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())
     }
 }

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<RawFd>,
+    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 _,
+                &self.prog_fd as *const _ as *const _,
                 mem::size_of::<RawFd>() as u32,
             );
         }
         Ok(())
-        } else {
-            Err(ProgramError::AlreadyDetached)
-        }
-    }
-}
-
-impl Drop for SocketFilterLink {
-    fn drop(&mut self) {
-        let _ = self.detach();
     }
 }

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<RawFd>,
     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)
-        }
     }
 }
 

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())
     }
 }

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<RawFd>,
+    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) };
+        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(())
-        } else {
-            Err(ProgramError::AlreadyDetached)
-        }
-    }
-}
-
-impl Drop for NlLink {
-    fn drop(&mut self) {
-        let _ = self.detach();
     }
 }

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<RawFd>,
-    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()?;