aya: Use Arc<OwnedFd> when loading BTF fd

This fixes an existing file descriptor leak when there is BTF data in
the loaded object.

To avoid lifetime issues while having minimal impact to UX the
`OwnedFd` returned from the BPF_BTF_LOAD syscall will be wrapped in an
`Arc` and shared accross the programs and maps of the loaded BPF
file.
pull/662/head
Andrés Medina 1 year ago
parent 683a1cf2e4
commit ea96c29ccb

@ -3,8 +3,12 @@ use std::{
collections::{HashMap, HashSet},
ffi::CString,
fs, io,
os::{fd::RawFd, raw::c_int},
os::{
fd::{OwnedFd, RawFd},
raw::c_int,
},
path::{Path, PathBuf},
sync::Arc,
};
use aya_obj::{
@ -390,7 +394,7 @@ impl<'a> BpfLoader<'a> {
let btf_fd = if let Some(features) = &FEATURES.btf() {
if let Some(btf) = obj.fixup_and_sanitize_btf(features)? {
match load_btf(btf.to_bytes(), *verifier_log_level) {
Ok(btf_fd) => Some(btf_fd),
Ok(btf_fd) => Some(Arc::new(btf_fd)),
// Only report an error here if the BTF is truely needed, otherwise proceed without.
Err(err) => {
for program in obj.programs.values() {
@ -473,7 +477,7 @@ impl<'a> BpfLoader<'a> {
obj,
fd: None,
pinned: false,
btf_fd,
btf_fd: btf_fd.as_ref().map(Arc::clone),
};
let fd = match map.obj.pinning() {
PinningType::ByName => {
@ -543,6 +547,7 @@ impl<'a> BpfLoader<'a> {
let section = prog_obj.section.clone();
let obj = (prog_obj, function_obj);
let btf_fd = btf_fd.as_ref().map(Arc::clone);
let program = if extensions.contains(name.as_str()) {
Program::Extension(Extension {
data: ProgramData::new(prog_name, obj, btf_fd, *verifier_log_level),
@ -993,17 +998,14 @@ pub enum BpfError {
ProgramError(#[from] ProgramError),
}
fn load_btf(raw_btf: Vec<u8>, verifier_log_level: VerifierLogLevel) -> Result<RawFd, BtfError> {
fn load_btf(raw_btf: Vec<u8>, verifier_log_level: VerifierLogLevel) -> Result<OwnedFd, BtfError> {
let (ret, verifier_log) = retry_with_verifier_logs(10, |logger| {
bpf_load_btf(raw_btf.as_slice(), logger, verifier_log_level)
});
match ret {
Ok(fd) => Ok(fd as RawFd),
Err((_, io_error)) => Err(BtfError::LoadError {
ret.map_err(|(_, io_error)| BtfError::LoadError {
io_error,
verifier_log,
}),
}
})
}
/// Global data that can be exported to eBPF programs before they are loaded.

@ -42,9 +42,10 @@ use std::{
marker::PhantomData,
mem,
ops::Deref,
os::fd::{AsRawFd, RawFd},
os::fd::{AsFd, AsRawFd, OwnedFd, RawFd},
path::Path,
ptr,
sync::Arc,
};
use crate::util::KernelVersion;
@ -486,7 +487,7 @@ pub(crate) fn check_v_size<V>(map: &MapData) -> Result<(), MapError> {
pub struct MapData {
pub(crate) obj: obj::Map,
pub(crate) fd: Option<RawFd>,
pub(crate) btf_fd: Option<RawFd>,
pub(crate) btf_fd: Option<Arc<OwnedFd>>,
/// Indicates if this map has been pinned to bpffs
pub pinned: bool,
}
@ -504,8 +505,13 @@ impl MapData {
let kernel_version = KernelVersion::current().unwrap();
#[cfg(test)]
let kernel_version = KernelVersion::new(0xff, 0xff, 0xff);
let fd = bpf_create_map(&c_name, &self.obj, self.btf_fd, kernel_version).map_err(
|(code, io_error)| {
let fd = bpf_create_map(
&c_name,
&self.obj,
self.btf_fd.as_ref().map(|f| f.as_fd()),
kernel_version,
)
.map_err(|(code, io_error)| {
if kernel_version < KernelVersion::new(5, 11, 0) {
maybe_warn_rlimit();
}
@ -515,8 +521,7 @@ impl MapData {
code,
io_error,
}
},
)? as RawFd;
})? as RawFd;
self.fd = Some(fd);
@ -639,7 +644,7 @@ impl Clone for MapData {
MapData {
obj: self.obj.clone(),
fd: self.fd.map(|fd| unsafe { libc::dup(fd) }),
btf_fd: self.btf_fd,
btf_fd: self.btf_fd.as_ref().map(Arc::clone),
pinned: self.pinned,
}
}

@ -69,8 +69,9 @@ use libc::ENOSPC;
use std::{
ffi::CString,
io,
os::fd::{AsRawFd, IntoRawFd as _, RawFd},
os::fd::{AsFd, AsRawFd, IntoRawFd as _, OwnedFd, RawFd},
path::{Path, PathBuf},
sync::Arc,
};
use thiserror::Error;
@ -413,7 +414,7 @@ pub(crate) struct ProgramData<T: Link> {
pub(crate) attach_btf_obj_fd: Option<u32>,
pub(crate) attach_btf_id: Option<u32>,
pub(crate) attach_prog_fd: Option<RawFd>,
pub(crate) btf_fd: Option<RawFd>,
pub(crate) btf_fd: Option<Arc<OwnedFd>>,
pub(crate) verifier_log_level: VerifierLogLevel,
pub(crate) path: Option<PathBuf>,
pub(crate) flags: u32,
@ -423,7 +424,7 @@ impl<T: Link> ProgramData<T> {
pub(crate) fn new(
name: Option<String>,
obj: (obj::Program, obj::Function),
btf_fd: Option<RawFd>,
btf_fd: Option<Arc<OwnedFd>>,
verifier_log_level: VerifierLogLevel,
) -> ProgramData<T> {
ProgramData {
@ -613,7 +614,7 @@ fn load_program<T: Link>(
license,
kernel_version: target_kernel_version,
expected_attach_type: *expected_attach_type,
prog_btf_fd: *btf_fd,
prog_btf_fd: btf_fd.as_ref().map(|f| f.as_fd()),
attach_btf_obj_fd: *attach_btf_obj_fd,
attach_btf_id: *attach_btf_id,
attach_prog_fd: *attach_prog_fd,

@ -3,7 +3,7 @@ use std::{
ffi::{CStr, CString},
io,
mem::{self, MaybeUninit},
os::fd::{FromRawFd as _, OwnedFd, RawFd},
os::fd::{AsRawFd, BorrowedFd, FromRawFd as _, OwnedFd, RawFd},
slice,
};
@ -35,7 +35,7 @@ use crate::{
pub(crate) fn bpf_create_map(
name: &CStr,
def: &obj::Map,
btf_fd: Option<RawFd>,
btf_fd: Option<BorrowedFd<'_>>,
kernel_version: KernelVersion,
) -> SysResult<c_long> {
let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
@ -75,7 +75,7 @@ pub(crate) fn bpf_create_map(
_ => {
u.btf_key_type_id = m.def.btf_key_type_id;
u.btf_value_type_id = m.def.btf_value_type_id;
u.btf_fd = btf_fd.unwrap_or_default() as u32;
u.btf_fd = btf_fd.map(|fd| fd.as_raw_fd()).unwrap_or_default() as u32;
}
}
}
@ -115,7 +115,7 @@ pub(crate) struct BpfLoadProgramAttrs<'a> {
pub(crate) license: &'a CStr,
pub(crate) kernel_version: u32,
pub(crate) expected_attach_type: Option<bpf_attach_type>,
pub(crate) prog_btf_fd: Option<RawFd>,
pub(crate) prog_btf_fd: Option<BorrowedFd<'a>>,
pub(crate) attach_btf_obj_fd: Option<u32>,
pub(crate) attach_btf_id: Option<u32>,
pub(crate) attach_prog_fd: Option<RawFd>,
@ -161,7 +161,7 @@ pub(crate) fn bpf_load_program(
let func_info_buf = aya_attr.func_info.func_info_bytes();
if let Some(btf_fd) = aya_attr.prog_btf_fd {
u.prog_btf_fd = btf_fd as u32;
u.prog_btf_fd = btf_fd.as_raw_fd() as u32;
if aya_attr.line_info_rec_size > 0 {
u.line_info = line_info_buf.as_ptr() as *const _ as u64;
u.line_info_cnt = aya_attr.line_info.len() as u32;
@ -464,21 +464,8 @@ pub(crate) fn bpf_prog_get_fd_by_id(prog_id: u32) -> Result<OwnedFd, io::Error>
let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
attr.__bindgen_anon_6.__bindgen_anon_1.prog_id = prog_id;
match sys_bpf(bpf_cmd::BPF_PROG_GET_FD_BY_ID, &attr) {
Ok(v) => {
let v = v.try_into().map_err(|_err| {
// _err is std::num::TryFromIntError or std::convert::Infallible depending on
// target, so we can't ascribe.
io::Error::new(
io::ErrorKind::InvalidData,
format!("bpf_prog_get_fd_by_id: invalid fd returned: {}", v),
)
})?;
Ok(unsafe { OwnedFd::from_raw_fd(v) })
}
Err((_, err)) => Err(err),
}
// SAFETY: BPF_PROG_GET_FD_BY_ID returns a new file descriptor.
unsafe { fd_sys_bpf(bpf_cmd::BPF_PROG_GET_FD_BY_ID, &attr).map_err(|(_, e)| e) }
}
pub(crate) fn bpf_prog_get_info_by_fd(prog_fd: RawFd) -> Result<bpf_prog_info, io::Error> {
@ -561,7 +548,7 @@ pub(crate) fn bpf_load_btf(
raw_btf: &[u8],
log_buf: &mut [u8],
verifier_log_level: VerifierLogLevel,
) -> SysResult<c_long> {
) -> SysResult<OwnedFd> {
let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
let u = unsafe { &mut attr.__bindgen_anon_7 };
u.btf = raw_btf.as_ptr() as *const _ as u64;
@ -571,7 +558,23 @@ pub(crate) fn bpf_load_btf(
u.btf_log_buf = log_buf.as_mut_ptr() as u64;
u.btf_log_size = log_buf.len() as u32;
}
sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr)
// SAFETY: `BPF_BTF_LOAD` returns a newly created fd.
unsafe { fd_sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr) }
}
// SAFETY: only use for bpf_cmd that return a new file descriptor on success.
unsafe fn fd_sys_bpf(cmd: bpf_cmd, attr: &bpf_attr) -> SysResult<OwnedFd> {
let fd = sys_bpf(cmd, attr)?;
let fd = fd.try_into().map_err(|_| {
(
fd,
io::Error::new(
io::ErrorKind::InvalidData,
format!("{cmd:?}: invalid fd returned: {fd}"),
),
)
})?;
Ok(OwnedFd::from_raw_fd(fd))
}
pub(crate) fn bpf_btf_get_fd_by_id(id: u32) -> Result<RawFd, io::Error> {

Loading…
Cancel
Save