Skip to content

Commit

Permalink
programs: ProgramFd is owned
Browse files Browse the repository at this point in the history
`ProgramData::fd` is now owned. This means that `ProgramData` now closes
the file descriptor on drop. In the future we might consider making
`ProgramFd` hold a `BorrowedFd` but this requires API design work due to
overlapping borrows.

Updates #612.
  • Loading branch information
tamird committed Aug 11, 2023
1 parent 03c5012 commit 5def006
Show file tree
Hide file tree
Showing 24 changed files with 149 additions and 118 deletions.
5 changes: 3 additions & 2 deletions aya/src/maps/array/program_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use std::{
borrow::{Borrow, BorrowMut},
os::fd::{AsRawFd, RawFd},
os::fd::{AsFd as _, AsRawFd as _, RawFd},
};

use crate::{
Expand Down Expand Up @@ -77,7 +77,8 @@ impl<T: BorrowMut<MapData>> ProgramArray<T> {
let data = self.inner.borrow_mut();
check_bounds(data, index)?;
let fd = data.fd_or_err()?;
let prog_fd = program.as_raw_fd();
let prog_fd = program.as_fd();
let prog_fd = prog_fd.as_raw_fd();

bpf_map_update_elem(fd, Some(&index), &prog_fd, flags).map_err(|(_, io_error)| {
SyscallError {
Expand Down
2 changes: 1 addition & 1 deletion aya/src/maps/perf/async_perf_event_array.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use bytes::BytesMut;
use std::{
borrow::{Borrow, BorrowMut},
os::fd::{AsRawFd, RawFd},
os::fd::{AsRawFd as _, RawFd},
};

// See https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features.
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/cgroup_device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl CgroupDevice {
/// The returned value can be used to detach, see [CgroupDevice::detach]
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupDeviceLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();

if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/cgroup_skb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ impl CgroupSkb {
attach_type: CgroupSkbAttachType,
) -> Result<CgroupSkbLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();

let attach_type = match attach_type {
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/cgroup_sock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl CgroupSock {
/// The returned value can be used to detach, see [CgroupSock::detach].
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSockLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
let attach_type = self.data.expected_attach_type.unwrap();
if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/cgroup_sock_addr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl CgroupSockAddr {
/// The returned value can be used to detach, see [CgroupSockAddr::detach].
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSockAddrLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
let attach_type = self.data.expected_attach_type.unwrap();
if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/cgroup_sockopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ impl CgroupSockopt {
/// The returned value can be used to detach, see [CgroupSockopt::detach].
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSockoptLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
let attach_type = self.data.expected_attach_type.unwrap();
if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/cgroup_sysctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl CgroupSysctl {
/// The returned value can be used to detach, see [CgroupSysctl::detach].
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSysctlLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();

if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {
Expand Down
28 changes: 18 additions & 10 deletions aya/src/programs/extension.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Extension programs.
use std::os::fd::{AsFd as _, AsRawFd as _, OwnedFd};
use std::os::fd::{AsFd as _, AsRawFd as _, BorrowedFd, OwnedFd};
use thiserror::Error;

use object::Endianness;
Expand Down Expand Up @@ -69,11 +69,11 @@ impl Extension {
/// 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_raw_fd();
let target_prog_fd = program.as_fd();
let (btf_fd, btf_id) = get_btf_info(target_prog_fd, func_name)?;

self.data.attach_btf_obj_fd = Some(btf_fd);
self.data.attach_prog_fd = Some(target_prog_fd);
self.data.attach_prog_fd = Some(target_prog_fd.as_raw_fd());
self.data.attach_btf_id = Some(btf_id);
load_program(BPF_PROG_TYPE_EXT, &mut self.data)
}
Expand All @@ -87,6 +87,7 @@ impl Extension {
/// original function, see [Extension::detach].
pub fn attach(&mut self) -> Result<ExtensionLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let target_fd = self.data.attach_prog_fd.ok_or(ProgramError::NotLoaded)?;
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
Expand Down Expand Up @@ -116,15 +117,22 @@ impl Extension {
program: ProgramFd,
func_name: &str,
) -> Result<ExtensionLinkId, ProgramError> {
let target_fd = program.as_raw_fd();
let target_fd = program.as_fd();
let (_, btf_id) = get_btf_info(target_fd, func_name)?;
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
// 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)
.map_err(|(_, io_error)| SyscallError {
call: "bpf_link_create",
io_error,
})?;
let link_fd = bpf_link_create(
prog_fd,
target_fd.as_raw_fd(),
BPF_CGROUP_INET_INGRESS,
Some(btf_id),
0,
)
.map_err(|(_, io_error)| SyscallError {
call: "bpf_link_create",
io_error,
})?;
self.data
.links
.insert(ExtensionLink::new(FdLink::new(link_fd)))
Expand All @@ -149,7 +157,7 @@ impl Extension {

/// Retrieves the FD of the BTF object for the provided `prog_fd` and the BTF ID of the function
/// with the name `func_name` within that BTF object.
fn get_btf_info(prog_fd: i32, func_name: &str) -> Result<(OwnedFd, u32), ProgramError> {
fn get_btf_info(prog_fd: BorrowedFd<'_>, func_name: &str) -> Result<(OwnedFd, u32), ProgramError> {
// retrieve program information
let info = sys::bpf_prog_get_info_by_fd(prog_fd, &mut [])?;

Expand Down
7 changes: 5 additions & 2 deletions aya/src/programs/lirc_mode2.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Lirc programs.
use std::os::fd::{AsRawFd, IntoRawFd as _, RawFd};
use std::os::fd::{AsRawFd, BorrowedFd, IntoRawFd as _, RawFd};

use crate::{
generated::{bpf_attach_type::BPF_LIRC_MODE2, bpf_prog_type::BPF_PROG_TYPE_LIRC_MODE2},
Expand Down Expand Up @@ -65,6 +65,7 @@ impl LircMode2 {
/// The returned value can be used to detach, see [LircMode2::detach].
pub fn attach<T: AsRawFd>(&mut self, lircdev: T) -> Result<LircLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let lircdev_fd = lircdev.as_raw_fd();

bpf_prog_attach(prog_fd, lircdev_fd, BPF_LIRC_MODE2).map_err(|(_, io_error)| {
Expand Down Expand Up @@ -131,7 +132,9 @@ impl LircLink {

/// Get ProgramInfo from this link
pub fn info(&self) -> Result<ProgramInfo, ProgramError> {
bpf_prog_get_info_by_fd(self.prog_fd, &mut [])
// 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) };
bpf_prog_get_info_by_fd(prog_fd, &mut [])
.map(ProgramInfo)
.map_err(Into::into)
}
Expand Down
54 changes: 30 additions & 24 deletions aya/src/programs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use libc::ENOSPC;
use std::{
ffi::CString,
io,
os::fd::{AsFd, AsRawFd, IntoRawFd as _, OwnedFd, RawFd},
os::fd::{AsFd, AsRawFd, BorrowedFd, IntoRawFd as _, OwnedFd, RawFd},
path::{Path, PathBuf},
sync::Arc,
};
Expand Down Expand Up @@ -211,12 +211,12 @@ pub enum ProgramError {
}

/// A [`Program`] file descriptor.
#[derive(Copy, Clone)]
pub struct ProgramFd(RawFd);
pub struct ProgramFd(OwnedFd);

impl AsRawFd for ProgramFd {
fn as_raw_fd(&self) -> RawFd {
self.0
impl AsFd for ProgramFd {
fn as_fd(&self) -> BorrowedFd<'_> {
let Self(fd) = self;
fd.as_fd()
}
}

Expand Down Expand Up @@ -369,7 +369,7 @@ impl 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) -> Option<ProgramFd> {
pub fn fd(&self) -> Result<ProgramFd, ProgramError> {
match self {
Program::KProbe(p) => p.fd(),
Program::UProbe(p) => p.fd(),
Expand Down Expand Up @@ -403,7 +403,7 @@ impl Program {
pub(crate) struct ProgramData<T: Link> {
pub(crate) name: Option<String>,
pub(crate) obj: Option<(obj::Program, obj::Function)>,
pub(crate) fd: Option<RawFd>,
pub(crate) fd: Option<OwnedFd>,
pub(crate) links: LinkMap<T>,
pub(crate) expected_attach_type: Option<bpf_attach_type>,
pub(crate) attach_btf_obj_fd: Option<OwnedFd>,
Expand Down Expand Up @@ -457,7 +457,7 @@ impl<T: Link> ProgramData<T> {
Ok(ProgramData {
name,
obj: None,
fd: Some(fd.into_raw_fd()),
fd: Some(fd),
links: LinkMap::new(),
expected_attach_type: None,
attach_btf_obj_fd,
Expand All @@ -481,15 +481,18 @@ impl<T: Link> ProgramData<T> {
io_error,
})?;

let info = bpf_prog_get_info_by_fd(fd.as_raw_fd(), &mut [])?;
let info = bpf_prog_get_info_by_fd(fd.as_fd(), &mut [])?;
let name = ProgramInfo(info).name_as_str().map(|s| s.to_string());
ProgramData::from_bpf_prog_info(name, fd, path.as_ref(), info, verifier_log_level)
}
}

impl<T: Link> ProgramData<T> {
fn fd_or_err(&self) -> Result<RawFd, ProgramError> {
self.fd.ok_or(ProgramError::NotLoaded)
fn fd_or_err(&self) -> Result<BorrowedFd<'_>, ProgramError> {
self.fd
.as_ref()
.map(AsFd::as_fd)
.ok_or(ProgramError::NotLoaded)
}

pub(crate) fn take_link(&mut self, link_id: T::Id) -> Result<T, ProgramError> {
Expand All @@ -499,15 +502,14 @@ impl<T: Link> ProgramData<T> {

fn unload_program<T: Link>(data: &mut ProgramData<T>) -> Result<(), ProgramError> {
data.links.remove_all()?;
let fd = data.fd.take().ok_or(ProgramError::NotLoaded)?;
unsafe {
libc::close(fd);
}
Ok(())
data.fd
.take()
.ok_or(ProgramError::NotLoaded)
.map(|_: OwnedFd| ())
}

fn pin_program<T: Link, P: AsRef<Path>>(data: &ProgramData<T>, path: P) -> Result<(), PinError> {
let fd = data.fd.ok_or(PinError::NoFd {
let fd = data.fd.as_ref().ok_or(PinError::NoFd {
name: data
.name
.as_deref()
Expand All @@ -519,7 +521,7 @@ fn pin_program<T: Link, P: AsRef<Path>>(data: &ProgramData<T>, path: P) -> Resul
error: e.to_string(),
}
})?;
bpf_pin_object(fd, &path_string).map_err(|(_, io_error)| SyscallError {
bpf_pin_object(fd.as_raw_fd(), &path_string).map_err(|(_, io_error)| SyscallError {
call: "BPF_OBJ_PIN",
io_error,
})?;
Expand Down Expand Up @@ -613,7 +615,7 @@ fn load_program<T: Link>(

match ret {
Ok(prog_fd) => {
*fd = Some(prog_fd as RawFd);
*fd = Some(prog_fd);
Ok(())
}
Err((_, io_error)) => Err(ProgramError::LoadError {
Expand Down Expand Up @@ -718,8 +720,12 @@ macro_rules! impl_fd {
$(
impl $struct_name {
/// Returns the file descriptor of this Program.
pub fn fd(&self) -> Option<ProgramFd> {
self.data.fd.map(|fd| ProgramFd(fd))
pub fn fd(&self) -> Result<ProgramFd, ProgramError> {
self.data.fd
.as_ref()
.ok_or(ProgramError::NotLoaded)
.and_then(|fd| fd.try_clone().map_err(Into::into))
.map(ProgramFd)
}
}
)+
Expand Down Expand Up @@ -948,7 +954,7 @@ impl ProgramInfo {
io_error,
})?;

let info = bpf_prog_get_info_by_fd(fd.as_raw_fd(), &mut [])?;
let info = bpf_prog_get_info_by_fd(fd.as_fd(), &mut [])?;
Ok(ProgramInfo(info))
}
}
Expand Down Expand Up @@ -984,7 +990,7 @@ pub fn loaded_programs() -> impl Iterator<Item = Result<ProgramInfo, ProgramErro
})
.map(|fd| {
let fd = fd?;
bpf_prog_get_info_by_fd(fd.as_raw_fd(), &mut [])
bpf_prog_get_info_by_fd(fd.as_fd(), &mut [])
})
.map(|result| result.map(ProgramInfo).map_err(Into::into))
}
Expand Down
6 changes: 4 additions & 2 deletions aya/src/programs/perf_event.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Perf event programs.
use std::os::fd::AsFd as _;
use std::os::fd::{AsFd as _, AsRawFd as _};

pub use crate::generated::{
perf_hw_cache_id, perf_hw_cache_op_id, perf_hw_cache_op_result_id, perf_hw_id, perf_sw_ids,
Expand Down Expand Up @@ -146,6 +146,8 @@ impl PerfEvent {
scope: PerfEventScope,
sample_policy: SamplePolicy,
) -> Result<PerfEventLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let (sample_period, sample_frequency) = match sample_policy {
SamplePolicy::Period(period) => (period, None),
SamplePolicy::Frequency(frequency) => (0, Some(frequency)),
Expand All @@ -172,7 +174,7 @@ impl PerfEvent {
io_error,
})?;

let link = perf_attach(self.data.fd_or_err()?, fd)?;
let link = perf_attach(prog_fd, fd)?;
self.data.links.insert(PerfEventLink::new(link))
}

Expand Down
23 changes: 10 additions & 13 deletions aya/src/programs/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use libc::pid_t;
use std::{
fs::{self, OpenOptions},
io::{self, Write},
os::fd::OwnedFd,
os::fd::{AsRawFd as _, OwnedFd},
path::Path,
process,
sync::atomic::{AtomicUsize, Ordering},
Expand Down Expand Up @@ -57,19 +57,16 @@ pub(crate) fn attach<T: Link + From<PerfLinkInner>>(
) -> Result<T::Id, ProgramError> {
// https://github.com/torvalds/linux/commit/e12f03d7031a977356e3d7b75a68c2185ff8d155
// Use debugfs to create probe
if KernelVersion::current().unwrap() < KernelVersion::new(4, 17, 0) {
let prog_fd = program_data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let link = if KernelVersion::current().unwrap() < KernelVersion::new(4, 17, 0) {
let (fd, event_alias) = create_as_trace_point(kind, fn_name, offset, pid)?;
let link = T::from(perf_attach_debugfs(
program_data.fd_or_err()?,
fd,
ProbeEvent { kind, event_alias },
)?);
return program_data.links.insert(link);
};

let fd = create_as_probe(kind, fn_name, offset, pid)?;
let link = T::from(perf_attach(program_data.fd_or_err()?, fd)?);
program_data.links.insert(link)
perf_attach_debugfs(prog_fd, fd, ProbeEvent { kind, event_alias })
} else {
let fd = create_as_probe(kind, fn_name, offset, pid)?;
perf_attach(prog_fd, fd)
}?;
program_data.links.insert(T::from(link))
}

pub(crate) fn detach_debug_fs(event: ProbeEvent) -> Result<(), ProgramError> {
Expand Down
1 change: 1 addition & 0 deletions aya/src/programs/sk_lookup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl SkLookup {
/// The returned value can be used to detach, see [SkLookup::detach].
pub fn attach<T: AsRawFd>(&mut self, netns: T) -> Result<SkLookupLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?;
let prog_fd = prog_fd.as_raw_fd();
let netns_fd = netns.as_raw_fd();

let link_fd = bpf_link_create(prog_fd, netns_fd, BPF_SK_LOOKUP, None, 0).map_err(
Expand Down
Loading

0 comments on commit 5def006

Please sign in to comment.