Browse Source

programs: `ProgramData::attach_prog_fd` is owned

This prevents a file descriptor leak when extensions are used.

This is an API breaking change.

Updates #612.
Tamir Duberstein 1 year ago
parent
commit
ae6526e59b

+ 11 - 6
aya/src/programs/extension.rs

@@ -44,7 +44,7 @@ pub enum ExtensionError {
 /// let prog_fd = prog.fd().unwrap();
 /// let prog_fd = prog_fd.try_clone().unwrap();
 /// let ext: &mut Extension = bpf.program_mut("extension").unwrap().try_into()?;
-/// ext.load(&prog_fd, "function_to_replace")?;
+/// ext.load(prog_fd, "function_to_replace")?;
 /// ext.attach()?;
 /// Ok::<(), aya::BpfError>(())
 /// ```
@@ -69,12 +69,11 @@ impl Extension {
     /// The extension code will be loaded but inactive until it's attached.
     /// There are no restrictions on what functions may be replaced, so you could replace
     /// the main entry point of your program with an extension.
-    pub fn load(&mut self, program: &ProgramFd, func_name: &str) -> Result<(), ProgramError> {
-        let target_prog_fd = program.as_fd();
-        let (btf_fd, btf_id) = get_btf_info(target_prog_fd, func_name)?;
+    pub fn load(&mut self, program: ProgramFd, func_name: &str) -> Result<(), ProgramError> {
+        let (btf_fd, btf_id) = get_btf_info(program.as_fd(), func_name)?;
 
         self.data.attach_btf_obj_fd = Some(btf_fd);
-        self.data.attach_prog_fd = Some(target_prog_fd.as_raw_fd());
+        self.data.attach_prog_fd = Some(program);
         self.data.attach_btf_id = Some(btf_id);
         load_program(BPF_PROG_TYPE_EXT, &mut self.data)
     }
@@ -90,7 +89,13 @@ impl Extension {
         let prog_fd = self.fd()?;
         let prog_fd = prog_fd.as_fd();
         let prog_fd = prog_fd.as_raw_fd();
-        let target_fd = self.data.attach_prog_fd.ok_or(ProgramError::NotLoaded)?;
+        let target_fd = self
+            .data
+            .attach_prog_fd
+            .as_ref()
+            .ok_or(ProgramError::NotLoaded)?;
+        let target_fd = target_fd.as_fd();
+        let target_fd = target_fd.as_raw_fd();
         let btf_id = self.data.attach_btf_id.ok_or(ProgramError::NotLoaded)?;
         // the attach type must be set as 0, which is bpf_attach_type::BPF_CGROUP_INET_INGRESS
         let link_fd = bpf_link_create(prog_fd, target_fd, BPF_CGROUP_INET_INGRESS, Some(btf_id), 0)

+ 3 - 4
aya/src/programs/mod.rs

@@ -69,7 +69,7 @@ use std::{
     ffi::CString,
     io,
     num::NonZeroU32,
-    os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd},
+    os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd},
     path::{Path, PathBuf},
     sync::Arc,
     time::{Duration, SystemTime},
@@ -382,7 +382,6 @@ impl Program {
     /// Returns the file descriptor of a program.
     ///
     /// Can be used to add a program to a [`crate::maps::ProgramArray`] or attach an [`Extension`] program.
-    /// Can be converted to [`RawFd`] using [`AsRawFd`].
     pub fn fd(&self) -> Result<&ProgramFd, ProgramError> {
         match self {
             Program::KProbe(p) => p.fd(),
@@ -422,7 +421,7 @@ pub(crate) struct ProgramData<T: Link> {
     pub(crate) expected_attach_type: Option<bpf_attach_type>,
     pub(crate) attach_btf_obj_fd: Option<OwnedFd>,
     pub(crate) attach_btf_id: Option<u32>,
-    pub(crate) attach_prog_fd: Option<RawFd>,
+    pub(crate) attach_prog_fd: Option<ProgramFd>,
     pub(crate) btf_fd: Option<Arc<OwnedFd>>,
     pub(crate) verifier_log_level: VerifierLogLevel,
     pub(crate) path: Option<PathBuf>,
@@ -613,7 +612,7 @@ fn load_program<T: Link>(
         prog_btf_fd: btf_fd.as_ref().map(|f| f.as_fd()),
         attach_btf_obj_fd: attach_btf_obj_fd.as_ref().map(|fd| fd.as_fd()),
         attach_btf_id: *attach_btf_id,
-        attach_prog_fd: *attach_prog_fd,
+        attach_prog_fd: attach_prog_fd.as_ref().map(|fd| fd.as_fd()),
         func_info_rec_size: *func_info_rec_size,
         func_info: func_info.clone(),
         line_info_rec_size: *line_info_rec_size,

+ 2 - 2
aya/src/sys/bpf.rs

@@ -120,7 +120,7 @@ pub(crate) struct BpfLoadProgramAttrs<'a> {
     pub(crate) prog_btf_fd: Option<BorrowedFd<'a>>,
     pub(crate) attach_btf_obj_fd: Option<BorrowedFd<'a>>,
     pub(crate) attach_btf_id: Option<u32>,
-    pub(crate) attach_prog_fd: Option<RawFd>,
+    pub(crate) attach_prog_fd: Option<BorrowedFd<'a>>,
     pub(crate) func_info_rec_size: usize,
     pub(crate) func_info: FuncSecInfo,
     pub(crate) line_info_rec_size: usize,
@@ -184,7 +184,7 @@ pub(crate) fn bpf_load_program(
         u.__bindgen_anon_1.attach_btf_obj_fd = v.as_raw_fd() as _;
     }
     if let Some(v) = aya_attr.attach_prog_fd {
-        u.__bindgen_anon_1.attach_prog_fd = v as u32;
+        u.__bindgen_anon_1.attach_prog_fd = v.as_raw_fd() as u32;
     }
 
     if let Some(v) = aya_attr.attach_btf_id {

+ 3 - 1
test/integration-test/src/tests/smoke.rs

@@ -63,7 +63,9 @@ fn extension() {
         .load(crate::EXT)
         .unwrap();
     let drop_: &mut Extension = bpf.program_mut("xdp_drop").unwrap().try_into().unwrap();
-    drop_.load(pass.fd().unwrap(), "xdp_pass").unwrap();
+    drop_
+        .load(pass.fd().unwrap().try_clone().unwrap(), "xdp_pass")
+        .unwrap();
 }
 
 #[test]

+ 2 - 2
xtask/public-api/aya.txt

@@ -2521,7 +2521,7 @@ impl aya::programs::extension::Extension
 pub fn aya::programs::extension::Extension::attach(&mut self) -> core::result::Result<aya::programs::extension::ExtensionLinkId, aya::programs::ProgramError>
 pub fn aya::programs::extension::Extension::attach_to_program(&mut self, program: &aya::programs::ProgramFd, func_name: &str) -> core::result::Result<aya::programs::extension::ExtensionLinkId, aya::programs::ProgramError>
 pub fn aya::programs::extension::Extension::detach(&mut self, link_id: aya::programs::extension::ExtensionLinkId) -> core::result::Result<(), aya::programs::ProgramError>
-pub fn aya::programs::extension::Extension::load(&mut self, program: &aya::programs::ProgramFd, func_name: &str) -> core::result::Result<(), aya::programs::ProgramError>
+pub fn aya::programs::extension::Extension::load(&mut self, program: aya::programs::ProgramFd, func_name: &str) -> core::result::Result<(), aya::programs::ProgramError>
 pub fn aya::programs::extension::Extension::take_link(&mut self, link_id: aya::programs::extension::ExtensionLinkId) -> core::result::Result<aya::programs::extension::ExtensionLink, aya::programs::ProgramError>
 impl aya::programs::extension::Extension
 pub fn aya::programs::extension::Extension::fd(&self) -> core::result::Result<&aya::programs::ProgramFd, aya::programs::ProgramError>
@@ -5855,7 +5855,7 @@ impl aya::programs::extension::Extension
 pub fn aya::programs::extension::Extension::attach(&mut self) -> core::result::Result<aya::programs::extension::ExtensionLinkId, aya::programs::ProgramError>
 pub fn aya::programs::extension::Extension::attach_to_program(&mut self, program: &aya::programs::ProgramFd, func_name: &str) -> core::result::Result<aya::programs::extension::ExtensionLinkId, aya::programs::ProgramError>
 pub fn aya::programs::extension::Extension::detach(&mut self, link_id: aya::programs::extension::ExtensionLinkId) -> core::result::Result<(), aya::programs::ProgramError>
-pub fn aya::programs::extension::Extension::load(&mut self, program: &aya::programs::ProgramFd, func_name: &str) -> core::result::Result<(), aya::programs::ProgramError>
+pub fn aya::programs::extension::Extension::load(&mut self, program: aya::programs::ProgramFd, func_name: &str) -> core::result::Result<(), aya::programs::ProgramError>
 pub fn aya::programs::extension::Extension::take_link(&mut self, link_id: aya::programs::extension::ExtensionLinkId) -> core::result::Result<aya::programs::extension::ExtensionLink, aya::programs::ProgramError>
 impl aya::programs::extension::Extension
 pub fn aya::programs::extension::Extension::fd(&self) -> core::result::Result<&aya::programs::ProgramFd, aya::programs::ProgramError>