programs: ProgAttachLink and LircLink hold owned FDs

Updates #612.

aya/src/programs/links.rs

@@ -12,10 +12,8 @@ use std::{
 use crate::{
     generated::bpf_attach_type,
     pin::PinError,
-    programs::ProgramError,
-    sys::{
-        bpf_get_object, bpf_pin_object, bpf_prog_attach, bpf_prog_detach, SysResult, SyscallError,
-    },
+    programs::{ProgramError, ProgramFd},
+    sys::{bpf_get_object, bpf_pin_object, bpf_prog_attach, bpf_prog_detach, SyscallError},
 };
 
 /// A Link.
@@ -234,7 +232,7 @@ pub struct ProgAttachLinkId(RawFd, RawFd, bpf_attach_type);
 /// The Link type used by programs that are attached with `bpf_prog_attach`.
 #[derive(Debug)]
 pub struct ProgAttachLink {
-    prog_fd: RawFd,
+    prog_fd: ProgramFd,
     target_fd: OwnedFd,
     attach_type: bpf_attach_type,
 }
@@ -249,15 +247,11 @@ impl ProgAttachLink {
         // going to need a duplicate whose lifetime we manage. Let's
         // duplicate it prior to attaching it so the new file
         // descriptor is closed at drop in case it fails to attach.
+        let prog_fd = prog_fd.try_clone_to_owned()?;
         let target_fd = target_fd.try_clone_to_owned()?;
-        bpf_prog_attach(prog_fd, target_fd.as_fd(), attach_type).map_err(|(_, io_error)| {
-            SyscallError {
-                call: "bpf_prog_attach",
-                io_error,
-            }
-        })?;
+        bpf_prog_attach(prog_fd.as_fd(), target_fd.as_fd(), attach_type)?;
 
-        let prog_fd = prog_fd.as_raw_fd();
+        let prog_fd = ProgramFd(prog_fd);
         Ok(Self {
             prog_fd,
             target_fd,
@@ -270,13 +264,20 @@ impl Link for ProgAttachLink {
     type Id = ProgAttachLinkId;
 
     fn id(&self) -> Self::Id {
-        ProgAttachLinkId(self.prog_fd, self.target_fd.as_raw_fd(), self.attach_type)
+        ProgAttachLinkId(
+            self.prog_fd.as_fd().as_raw_fd(),
+            self.target_fd.as_raw_fd(),
+            self.attach_type,
+        )
     }
 
     fn detach(self) -> Result<(), ProgramError> {
-        let _: SysResult<_> =
-            bpf_prog_detach(self.prog_fd, self.target_fd.as_fd(), self.attach_type);
-        Ok(())
+        bpf_prog_detach(
+            self.prog_fd.as_fd(),
+            self.target_fd.as_fd(),
+            self.attach_type,
+        )
+        .map_err(Into::into)
     }
 }
 

aya/src/programs/lirc_mode2.rs

@@ -1,10 +1,10 @@
 //! Lirc programs.
-use std::os::fd::{AsFd, AsRawFd, BorrowedFd, IntoRawFd as _, OwnedFd, RawFd};
+use std::os::fd::{AsFd, AsRawFd as _, OwnedFd, RawFd};
 
 use crate::{
     generated::{bpf_attach_type::BPF_LIRC_MODE2, bpf_prog_type::BPF_PROG_TYPE_LIRC_MODE2},
-    programs::{load_program, query, Link, ProgramData, ProgramError, ProgramInfo},
-    sys::{bpf_prog_attach, bpf_prog_detach, bpf_prog_get_fd_by_id, SysResult, SyscallError},
+    programs::{load_program, query, Link, ProgramData, ProgramError, ProgramFd, ProgramInfo},
+    sys::{bpf_prog_attach, bpf_prog_detach, bpf_prog_get_fd_by_id},
 };
 
 /// A program used to decode IR into key events for a lirc device.
@@ -60,20 +60,15 @@ impl LircMode2 {
     /// The returned value can be used to detach, see [LircMode2::detach].
     pub fn attach<T: AsFd>(&mut self, lircdev: T) -> Result<LircLinkId, ProgramError> {
         let prog_fd = self.fd()?;
-        let prog_fd = prog_fd.as_fd();
 
         // The link is going to own this new file descriptor so we are
         // going to need a duplicate whose lifetime we manage. Let's
         // duplicate it prior to attaching it so the new file
         // descriptor is closed at drop in case it fails to attach.
+        let prog_fd = prog_fd.try_clone()?;
         let lircdev_fd = lircdev.as_fd().try_clone_to_owned()?;
 
-        bpf_prog_attach(prog_fd, lircdev_fd.as_fd(), BPF_LIRC_MODE2).map_err(|(_, io_error)| {
-            SyscallError {
-                call: "bpf_prog_attach",
-                io_error,
-            }
-        })?;
+        bpf_prog_attach(prog_fd.as_fd(), lircdev_fd.as_fd(), BPF_LIRC_MODE2)?;
 
         self.data.links.insert(LircLink::new(prog_fd, lircdev_fd))
     }
@@ -103,13 +98,7 @@ impl LircMode2 {
             .map(|prog_id| {
                 let prog_fd = bpf_prog_get_fd_by_id(prog_id)?;
                 let target_fd = target_fd.try_clone_to_owned()?;
-                // SAFETY: The file descriptor will stay valid because
-                // we are leaking it. We cannot use `OwnedFd` in here
-                // because LircMode2::attach also uses LircLink::new
-                // but with a borrowed file descriptor (of the loaded
-                // program) without duplicating. TODO(#612): Fix API
-                // or internals so this file descriptor isn't leaked
-                let prog_fd = unsafe { BorrowedFd::borrow_raw(prog_fd.into_raw_fd()) };
+                let prog_fd = ProgramFd(prog_fd);
                 Ok(LircLink::new(prog_fd, target_fd))
             })
             .collect()
@@ -123,21 +112,22 @@ pub struct LircLinkId(RawFd, RawFd);
 #[derive(Debug)]
 /// An LircMode2 Link
 pub struct LircLink {
-    prog_fd: RawFd,
+    prog_fd: ProgramFd,
     target_fd: OwnedFd,
 }
 
 impl LircLink {
-    pub(crate) fn new(prog_fd: BorrowedFd<'_>, target_fd: OwnedFd) -> Self {
-        let prog_fd = prog_fd.as_raw_fd();
+    pub(crate) fn new(prog_fd: ProgramFd, target_fd: OwnedFd) -> Self {
         Self { prog_fd, target_fd }
     }
 
     /// Get ProgramInfo from this link
     pub fn info(&self) -> Result<ProgramInfo, ProgramError> {
-        // SAFETY: TODO(https://github.com/aya-rs/aya/issues/612): make this safe by not holding `RawFd`s.
-        let prog_fd = unsafe { BorrowedFd::borrow_raw(self.prog_fd) };
-        ProgramInfo::new_from_fd(prog_fd)
+        let Self {
+            prog_fd,
+            target_fd: _,
+        } = self;
+        ProgramInfo::new_from_fd(prog_fd.as_fd())
     }
 }
 
@@ -145,11 +135,11 @@ impl Link for LircLink {
     type Id = LircLinkId;
 
     fn id(&self) -> Self::Id {
-        LircLinkId(self.prog_fd, self.target_fd.as_raw_fd())
+        LircLinkId(self.prog_fd.as_fd().as_raw_fd(), self.target_fd.as_raw_fd())
     }
 
     fn detach(self) -> Result<(), ProgramError> {
-        let _: SysResult<_> = bpf_prog_detach(self.prog_fd, self.target_fd.as_fd(), BPF_LIRC_MODE2);
-        Ok(())
+        bpf_prog_detach(self.prog_fd.as_fd(), self.target_fd.as_fd(), BPF_LIRC_MODE2)
+            .map_err(Into::into)
     }
 }

aya/src/sys/bpf.rs

@@ -425,28 +425,44 @@ pub(crate) fn bpf_prog_attach(
     prog_fd: BorrowedFd<'_>,
     target_fd: BorrowedFd<'_>,
     attach_type: bpf_attach_type,
-) -> SysResult<c_long> {
+) -> Result<(), SyscallError> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     attr.__bindgen_anon_5.attach_bpf_fd = prog_fd.as_raw_fd() as u32;
     attr.__bindgen_anon_5.target_fd = target_fd.as_raw_fd() as u32;
     attr.__bindgen_anon_5.attach_type = attach_type as u32;
 
-    sys_bpf(bpf_cmd::BPF_PROG_ATTACH, &mut attr)
+    let ret = sys_bpf(bpf_cmd::BPF_PROG_ATTACH, &mut attr).map_err(|(code, io_error)| {
+        assert_eq!(code, -1);
+        SyscallError {
+            call: "bpf_prog_attach",
+            io_error,
+        }
+    })?;
+    assert_eq!(ret, 0);
+    Ok(())
 }
 
 pub(crate) fn bpf_prog_detach(
-    prog_fd: RawFd,
+    prog_fd: BorrowedFd<'_>,
     target_fd: BorrowedFd<'_>,
     attach_type: bpf_attach_type,
-) -> SysResult<c_long> {
+) -> Result<(), SyscallError> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
-    attr.__bindgen_anon_5.attach_bpf_fd = prog_fd as u32;
+    attr.__bindgen_anon_5.attach_bpf_fd = prog_fd.as_raw_fd() as u32;
     attr.__bindgen_anon_5.target_fd = target_fd.as_raw_fd() as u32;
     attr.__bindgen_anon_5.attach_type = attach_type as u32;
 
-    sys_bpf(bpf_cmd::BPF_PROG_DETACH, &mut attr)
+    let ret = sys_bpf(bpf_cmd::BPF_PROG_DETACH, &mut attr).map_err(|(code, io_error)| {
+        assert_eq!(code, -1);
+        SyscallError {
+            call: "bpf_prog_detach",
+            io_error,
+        }
+    })?;
+    assert_eq!(ret, 0);
+    Ok(())
 }
 
 pub(crate) fn bpf_prog_query(