aya: Use OwnedFd for Program related actions

This commit reveals but does not address a file descriptor leak in
LircLink2::query. This function returns a list of `LircLink`s where
each of them have a program file descriptor that is not going to be
closed. This commit does not add this leak; it merely makes it louder
in the code.
reviewable/pr723/r8
Andrés Medina 2 years ago
parent 45df2519b6
commit 02af9a9f24

@ -6,7 +6,7 @@ use std::{
collections::{hash_map::Entry, HashMap},
ffi::CString,
io,
os::fd::{AsRawFd as _, OwnedFd, RawFd},
os::fd::{AsRawFd as _, BorrowedFd, OwnedFd, RawFd},
path::{Path, PathBuf},
};
@ -237,12 +237,12 @@ pub struct ProgAttachLink {
impl ProgAttachLink {
pub(crate) fn new(
prog_fd: RawFd,
prog_fd: BorrowedFd<'_>,
target_fd: RawFd,
attach_type: bpf_attach_type,
) -> ProgAttachLink {
ProgAttachLink {
prog_fd,
prog_fd: prog_fd.as_raw_fd(),
target_fd: unsafe { dup(target_fd) },
attach_type,
}

@ -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},
@ -105,7 +105,14 @@ impl LircMode2 {
Ok(prog_fds
.into_iter()
.map(|prog_fd| LircLink::new(prog_fd.into_raw_fd(), target_fd.as_raw_fd()))
.map(|prog_fd| {
LircLink::new(
// SAFETY: The file descriptor will stay valid because we are leaking it
// TODO: Do not leak it?
unsafe { BorrowedFd::borrow_raw(prog_fd.into_raw_fd()) },
target_fd.as_raw_fd(),
)
})
.collect())
}
}
@ -122,9 +129,9 @@ pub struct LircLink {
}
impl LircLink {
pub(crate) fn new(prog_fd: RawFd, target_fd: RawFd) -> LircLink {
pub(crate) fn new(prog_fd: BorrowedFd<'_>, target_fd: RawFd) -> LircLink {
LircLink {
prog_fd,
prog_fd: prog_fd.as_raw_fd(),
target_fd: unsafe { dup(target_fd) },
}
}

@ -68,7 +68,7 @@ use libc::ENOSPC;
use std::{
ffi::CString,
io,
os::fd::{AsFd, AsRawFd, IntoRawFd as _, OwnedFd, RawFd},
os::fd::{AsFd as _, AsRawFd, BorrowedFd, OwnedFd, RawFd},
path::{Path, PathBuf},
sync::Arc,
};
@ -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<u32>,
@ -464,7 +464,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,
@ -495,8 +495,11 @@ impl<T: Link> ProgramData<T> {
}
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(|f| f.as_fd())
.ok_or(ProgramError::NotLoaded)
}
pub(crate) fn take_link(&mut self, link_id: T::Id) -> Result<T, ProgramError> {
@ -506,15 +509,12 @@ 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);
}
let _: OwnedFd = data.fd.take().ok_or(ProgramError::NotLoaded)?;
Ok(())
}
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()
@ -526,7 +526,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,
})?;
@ -620,7 +620,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 {
@ -726,7 +726,7 @@ 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))
self.data.fd.as_ref().map(|fd| ProgramFd(fd.as_raw_fd()))
}
}
)+
@ -939,12 +939,9 @@ impl ProgramInfo {
}
/// Returns the fd associated with the program.
///
/// The returned fd must be closed when no longer needed.
pub fn fd(&self) -> Result<RawFd, ProgramError> {
pub fn fd(&self) -> Result<OwnedFd, ProgramError> {
let Self(info) = self;
let fd = bpf_prog_get_fd_by_id(info.id)?;
Ok(fd.into_raw_fd())
bpf_prog_get_fd_by_id(info.id).map_err(Into::into)
}
/// Loads a program from a pinned path in bpffs.

@ -1,5 +1,5 @@
//! Perf attach links.
use std::os::fd::{AsFd as _, AsRawFd as _, OwnedFd, RawFd};
use std::os::fd::{AsFd as _, AsRawFd as _, BorrowedFd, OwnedFd, RawFd};
use crate::{
generated::bpf_attach_type::BPF_PERF_EVENT,
@ -70,7 +70,10 @@ impl Link for PerfLink {
}
}
pub(crate) fn perf_attach(prog_fd: RawFd, fd: OwnedFd) -> Result<PerfLinkInner, ProgramError> {
pub(crate) fn perf_attach(
prog_fd: std::os::fd::BorrowedFd<'_>,
fd: OwnedFd,
) -> Result<PerfLinkInner, ProgramError> {
if FEATURES.bpf_perf_link() {
let link_fd = bpf_link_create(prog_fd, fd.as_raw_fd(), BPF_PERF_EVENT, None, 0).map_err(
|(_, io_error)| SyscallError {
@ -85,7 +88,7 @@ pub(crate) fn perf_attach(prog_fd: RawFd, fd: OwnedFd) -> Result<PerfLinkInner,
}
pub(crate) fn perf_attach_debugfs(
prog_fd: RawFd,
prog_fd: BorrowedFd<'_>,
fd: OwnedFd,
event: ProbeEvent,
) -> Result<PerfLinkInner, ProgramError> {
@ -93,16 +96,16 @@ pub(crate) fn perf_attach_debugfs(
}
fn perf_attach_either(
prog_fd: RawFd,
prog_fd: BorrowedFd<'_>,
fd: OwnedFd,
event: Option<ProbeEvent>,
) -> Result<PerfLinkInner, ProgramError> {
perf_event_ioctl(fd.as_fd(), PERF_EVENT_IOC_SET_BPF, prog_fd).map_err(|(_, io_error)| {
SyscallError {
perf_event_ioctl(fd.as_fd(), PERF_EVENT_IOC_SET_BPF, prog_fd.as_raw_fd()).map_err(
|(_, io_error)| SyscallError {
call: "PERF_EVENT_IOC_SET_BPF",
io_error,
}
})?;
},
)?;
perf_event_ioctl(fd.as_fd(), PERF_EVENT_IOC_ENABLE, 0).map_err(|(_, io_error)| {
SyscallError {
call: "PERF_EVENT_IOC_ENABLE",

@ -1,5 +1,5 @@
//! Socket option programs.
use std::os::fd::AsRawFd;
use std::os::fd::{AsFd as _, AsRawFd};
use crate::{
generated::{bpf_attach_type::BPF_CGROUP_SOCK_OPS, bpf_prog_type::BPF_PROG_TYPE_SOCK_OPS},
@ -69,7 +69,7 @@ impl SockOps {
}
})?;
self.data.links.insert(SockOpsLink::new(ProgAttachLink::new(
prog_fd,
prog_fd.as_fd(),
cgroup_fd,
BPF_CGROUP_SOCK_OPS,
)))

@ -92,7 +92,10 @@ impl SocketFilter {
.into());
}
self.data.links.insert(SocketFilterLink { socket, prog_fd })
self.data.links.insert(SocketFilterLink {
socket,
prog_fd: prog_fd.as_raw_fd(),
})
}
/// Detaches the program.

@ -8,7 +8,7 @@ use std::{
ffi::CString,
hash::Hash,
io,
os::fd::{AsFd as _, RawFd},
os::fd::{AsFd as _, AsRawFd as _, RawFd},
};
use thiserror::Error;
@ -142,14 +142,14 @@ impl Xdp {
.links
.insert(XdpLink::new(XdpLinkInner::FdLink(FdLink::new(link_fd))))
} else {
unsafe { netlink_set_xdp_fd(if_index, prog_fd, None, flags.bits()) }
unsafe { netlink_set_xdp_fd(if_index, Some(prog_fd), None, flags.bits()) }
.map_err(|io_error| XdpError::NetlinkError { io_error })?;
self.data
.links
.insert(XdpLink::new(XdpLinkInner::NlLink(NlLink {
if_index,
prog_fd,
prog_fd: prog_fd.as_raw_fd(),
flags,
})))
}
@ -178,12 +178,12 @@ impl Xdp {
match link.into_inner() {
XdpLinkInner::FdLink(fd_link) => {
let link_fd = fd_link.fd;
bpf_link_update(link_fd.as_fd(), prog_fd, None, 0).map_err(|(_, io_error)| {
SyscallError {
bpf_link_update(link_fd.as_fd(), prog_fd.as_fd(), None, 0).map_err(
|(_, io_error)| SyscallError {
call: "bpf_link_update",
io_error,
}
})?;
},
)?;
self.data
.links
@ -195,7 +195,12 @@ impl Xdp {
let flags = nl_link.flags;
let replace_flags = flags | XdpFlags::REPLACE;
unsafe {
netlink_set_xdp_fd(if_index, prog_fd, Some(old_prog_fd), replace_flags.bits())
netlink_set_xdp_fd(
if_index,
Some(prog_fd.as_fd()),
Some(old_prog_fd),
replace_flags.bits(),
)
.map_err(|io_error| XdpError::NetlinkError { io_error })?;
}
@ -203,7 +208,7 @@ impl Xdp {
.links
.insert(XdpLink::new(XdpLinkInner::NlLink(NlLink {
if_index,
prog_fd,
prog_fd: prog_fd.as_raw_fd(),
flags,
})))
}
@ -231,7 +236,7 @@ impl Link for NlLink {
} else {
self.flags.bits()
};
let _ = unsafe { netlink_set_xdp_fd(self.if_index, -1, Some(self.prog_fd), flags) };
let _ = unsafe { netlink_set_xdp_fd(self.if_index, None, Some(self.prog_fd), flags) };
Ok(())
}
}

@ -3,12 +3,12 @@ use std::{
ffi::{CStr, CString},
io, iter,
mem::{self, MaybeUninit},
os::fd::{AsRawFd, BorrowedFd, FromRawFd as _, OwnedFd, RawFd},
os::fd::{AsFd as _, AsRawFd as _, BorrowedFd, FromRawFd as _, OwnedFd, RawFd},
slice,
};
use crate::util::KernelVersion;
use libc::{c_char, c_long, close, ENOENT, ENOSPC};
use libc::{c_char, c_long, ENOENT, ENOSPC};
use obj::{
btf::{BtfEnum64, Enum64},
maps::{bpf_map_def, LegacyMap},
@ -132,7 +132,7 @@ pub(crate) fn bpf_load_program(
aya_attr: &BpfLoadProgramAttrs,
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_3 };
@ -190,7 +190,8 @@ pub(crate) fn bpf_load_program(
if let Some(v) = aya_attr.attach_btf_id {
u.attach_btf_id = v;
}
sys_bpf(bpf_cmd::BPF_PROG_LOAD, &mut attr)
bpf_prog_load(&mut attr)
}
fn lookup<K: Pod, V: Pod>(
@ -367,7 +368,7 @@ pub(crate) fn bpf_map_freeze(fd: RawFd) -> SysResult<c_long> {
// since kernel 5.7
pub(crate) fn bpf_link_create(
prog_fd: RawFd,
prog_fd: BorrowedFd<'_>,
target_fd: RawFd,
attach_type: bpf_attach_type,
btf_id: Option<u32>,
@ -375,7 +376,7 @@ pub(crate) fn bpf_link_create(
) -> SysResult<OwnedFd> {
let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
attr.link_create.__bindgen_anon_1.prog_fd = prog_fd as u32;
attr.link_create.__bindgen_anon_1.prog_fd = prog_fd.as_raw_fd() as u32;
attr.link_create.__bindgen_anon_2.target_fd = target_fd as u32;
attr.link_create.attach_type = attach_type as u32;
attr.link_create.flags = flags;
@ -390,14 +391,14 @@ pub(crate) fn bpf_link_create(
// since kernel 5.7
pub(crate) fn bpf_link_update(
link_fd: BorrowedFd<'_>,
new_prog_fd: RawFd,
new_prog_fd: BorrowedFd<'_>,
old_prog_fd: Option<RawFd>,
flags: u32,
) -> SysResult<c_long> {
let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
attr.link_update.link_fd = link_fd.as_raw_fd() as u32;
attr.link_update.__bindgen_anon_1.new_prog_fd = new_prog_fd as u32;
attr.link_update.__bindgen_anon_1.new_prog_fd = new_prog_fd.as_raw_fd() as u32;
if let Some(fd) = old_prog_fd {
attr.link_update.__bindgen_anon_2.old_prog_fd = fd as u32;
attr.link_update.flags = flags | BPF_F_REPLACE;
@ -409,13 +410,13 @@ pub(crate) fn bpf_link_update(
}
pub(crate) fn bpf_prog_attach(
prog_fd: RawFd,
prog_fd: BorrowedFd<'_>,
target_fd: RawFd,
attach_type: bpf_attach_type,
) -> SysResult<c_long> {
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 u32;
attr.__bindgen_anon_5.attach_type = attach_type as u32;
@ -547,14 +548,17 @@ pub(crate) fn btf_obj_get_info_by_fd(
}
}
pub(crate) fn bpf_raw_tracepoint_open(name: Option<&CStr>, prog_fd: RawFd) -> SysResult<OwnedFd> {
pub(crate) fn bpf_raw_tracepoint_open(
name: Option<&CStr>,
prog_fd: BorrowedFd<'_>,
) -> SysResult<OwnedFd> {
let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
attr.raw_tracepoint.name = match name {
Some(n) => n.as_ptr() as u64,
None => 0,
};
attr.raw_tracepoint.prog_fd = prog_fd as u32;
attr.raw_tracepoint.prog_fd = prog_fd.as_raw_fd() as u32;
// SAFETY: BPF_RAW_TRACEPOINT_OPEN returns a new file descriptor.
unsafe { fd_sys_bpf(bpf_cmd::BPF_RAW_TRACEPOINT_OPEN, &mut attr) }
@ -628,14 +632,7 @@ pub(crate) fn is_prog_name_supported() -> bool {
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_SOCKET_FILTER as u32;
match sys_bpf(bpf_cmd::BPF_PROG_LOAD, &mut attr) {
Ok(v) => {
let fd = v as RawFd;
unsafe { close(fd) };
true
}
Err(_) => false,
}
bpf_prog_load(&mut attr).is_ok()
}
pub(crate) fn is_probe_read_kernel_supported() -> bool {
@ -659,14 +656,7 @@ pub(crate) fn is_probe_read_kernel_supported() -> bool {
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_TRACEPOINT as u32;
match sys_bpf(bpf_cmd::BPF_PROG_LOAD, &mut attr) {
Ok(v) => {
let fd = v as RawFd;
unsafe { close(fd) };
true
}
Err(_) => false,
}
bpf_prog_load(&mut attr).is_ok()
}
pub(crate) fn is_perf_link_supported() -> bool {
@ -686,17 +676,16 @@ pub(crate) fn is_perf_link_supported() -> bool {
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_TRACEPOINT as u32;
if let Ok(fd) = sys_bpf(bpf_cmd::BPF_PROG_LOAD, &mut attr) {
let Ok(fd) = bpf_prog_load(&mut attr) else { return false };
if let Err((_, e)) =
// Uses an invalid target FD so we get EBADF if supported.
bpf_link_create(fd as i32, -1, bpf_attach_type::BPF_PERF_EVENT, None, 0)
bpf_link_create(fd.as_fd(), -1, bpf_attach_type::BPF_PERF_EVENT, None, 0)
{
// Returns EINVAL if unsupported. EBADF if supported.
let res = e.raw_os_error() == Some(libc::EBADF);
unsafe { libc::close(fd as i32) };
return res;
}
return e.raw_os_error() == Some(libc::EBADF);
}
false
}
@ -733,7 +722,8 @@ pub(crate) fn is_bpf_global_data_supported() -> bool {
btf_fd: None,
};
if let Ok(map_fd) = map_data.create("aya_global") {
let Ok(map_fd) = map_data.create("aya_global") else { return false };
insns[0].imm = map_fd;
let gpl = b"GPL\0";
@ -742,16 +732,7 @@ pub(crate) fn is_bpf_global_data_supported() -> bool {
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_SOCKET_FILTER as u32;
if let Ok(v) = sys_bpf(bpf_cmd::BPF_PROG_LOAD, &mut attr) {
let fd = v as RawFd;
unsafe { close(fd) };
return true;
}
}
false
bpf_prog_load(&mut attr).is_ok()
}
pub(crate) fn is_bpf_cookie_supported() -> bool {
@ -771,14 +752,7 @@ pub(crate) fn is_bpf_cookie_supported() -> bool {
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_KPROBE as u32;
match sys_bpf(bpf_cmd::BPF_PROG_LOAD, &mut attr) {
Ok(v) => {
let fd = v as RawFd;
unsafe { close(fd) };
true
}
Err(_) => false,
}
bpf_prog_load(&mut attr).is_ok()
}
pub(crate) fn is_btf_supported() -> bool {
@ -937,6 +911,11 @@ pub(crate) fn is_btf_type_tag_supported() -> bool {
bpf_load_btf(btf_bytes.as_slice(), &mut [], Default::default()).is_ok()
}
fn bpf_prog_load(attr: &mut bpf_attr) -> SysResult<OwnedFd> {
// SAFETY: BPF_PROG_LOAD returns a new file descriptor.
unsafe { fd_sys_bpf(bpf_cmd::BPF_PROG_LOAD, attr) }
}
fn sys_bpf(cmd: bpf_cmd, attr: &mut bpf_attr) -> SysResult<c_long> {
syscall(Syscall::Bpf { cmd, attr })
}

@ -1,4 +1,10 @@
use std::{collections::HashMap, ffi::CStr, io, mem, os::fd::RawFd, ptr, slice};
use std::{
collections::HashMap,
ffi::CStr,
io, mem,
os::fd::{AsRawFd as _, BorrowedFd, RawFd},
ptr, slice,
};
use thiserror::Error;
use libc::{
@ -25,7 +31,7 @@ const NLA_HDR_LEN: usize = align_to(mem::size_of::<nlattr>(), NLA_ALIGNTO as usi
// netlink alignments
pub(crate) unsafe fn netlink_set_xdp_fd(
if_index: i32,
fd: RawFd,
fd: Option<BorrowedFd<'_>>,
old_fd: Option<RawFd>,
flags: u32,
) -> Result<(), io::Error> {
@ -48,7 +54,10 @@ pub(crate) unsafe fn netlink_set_xdp_fd(
// write the attrs
let attrs_buf = request_attributes(&mut req, nlmsg_len);
let mut attrs = NestedAttrs::new(attrs_buf, IFLA_XDP);
attrs.write_attr(IFLA_XDP_FD as u16, fd)?;
attrs.write_attr(
IFLA_XDP_FD as u16,
fd.map(|fd| fd.as_raw_fd()).unwrap_or(-1),
)?;
if flags > 0 {
attrs.write_attr(IFLA_XDP_FLAGS as u16, flags)?;
@ -101,7 +110,7 @@ pub(crate) unsafe fn netlink_qdisc_add_clsact(if_index: i32) -> Result<(), io::E
pub(crate) unsafe fn netlink_qdisc_attach(
if_index: i32,
attach_type: &TcAttachType,
prog_fd: RawFd,
prog_fd: BorrowedFd<'_>,
prog_name: &CStr,
priority: u16,
handle: u32,

@ -6202,7 +6202,7 @@ impl<T> core::convert::From<T> for aya::programs::ProgramFd
pub fn aya::programs::ProgramFd::from(t: T) -> T
pub struct aya::programs::ProgramInfo(_)
impl aya::programs::ProgramInfo
pub fn aya::programs::ProgramInfo::fd(&self) -> core::result::Result<std::os::fd::raw::RawFd, aya::programs::ProgramError>
pub fn aya::programs::ProgramInfo::fd(&self) -> core::result::Result<std::os::fd::owned::OwnedFd, aya::programs::ProgramError>
pub fn aya::programs::ProgramInfo::from_pin<P: core::convert::AsRef<std::path::Path>>(path: P) -> core::result::Result<aya::programs::ProgramInfo, aya::programs::ProgramError>
pub fn aya::programs::ProgramInfo::id(&self) -> u32
pub fn aya::programs::ProgramInfo::name(&self) -> &[u8]

Loading…
Cancel
Save