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/r14
Andrés Medina 2 years ago
parent c7b5cd5eb5
commit 4fb558d3a4

@ -63,7 +63,6 @@ impl CgroupDevice {
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupDeviceLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {

@ -94,7 +94,6 @@ impl CgroupSkb {
) -> Result<CgroupSkbLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
let attach_type = match attach_type {

@ -73,7 +73,6 @@ impl CgroupSock {
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSockLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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) {

@ -74,7 +74,6 @@ impl CgroupSockAddr {
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSockAddrLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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) {

@ -71,7 +71,6 @@ impl CgroupSockopt {
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSockoptLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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) {

@ -65,7 +65,6 @@ impl CgroupSysctl {
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<CgroupSysctlLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
if KernelVersion::current().unwrap() >= KernelVersion::new(5, 7, 0) {

@ -86,9 +86,8 @@ impl Extension {
/// The returned value can be used to detach the extension and restore the
/// original function, see [Extension::detach].
pub fn attach(&mut self) -> Result<ExtensionLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = self.data.fd.as_ref().ok_or(ProgramError::NotLoaded)?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let target_fd = self
.data
.attach_prog_fd
@ -128,7 +127,6 @@ impl Extension {
let (_, btf_id) = get_btf_info(target_fd, func_name)?;
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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,

@ -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,
}

@ -63,7 +63,6 @@ impl LircMode2 {
pub fn attach<T: AsRawFd>(&mut self, lircdev: T) -> Result<LircLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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)| {
@ -104,7 +103,18 @@ 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. 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
unsafe { BorrowedFd::borrow_raw(prog_fd.into_raw_fd()) },
target_fd.as_raw_fd(),
)
})
.collect())
}
}
@ -121,9 +131,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) },
}
}

@ -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,6 +1,6 @@
//! Perf event programs.
use std::os::fd::{AsFd as _, AsRawFd as _};
use std::os::fd::AsFd 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,
@ -148,7 +148,6 @@ impl PerfEvent {
) -> Result<PerfEventLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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)),

@ -3,7 +3,7 @@ use libc::pid_t;
use std::{
fs::{self, OpenOptions},
io::{self, Write},
os::fd::{AsFd as _, AsRawFd as _, OwnedFd},
os::fd::{AsFd as _, OwnedFd},
path::Path,
process,
sync::atomic::{AtomicUsize, Ordering},
@ -59,7 +59,6 @@ pub(crate) fn attach<T: Link + From<PerfLinkInner>>(
// Use debugfs to create probe
let prog_fd = program_data.fd()?;
let prog_fd = prog_fd.as_fd();
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)?;
perf_attach_debugfs(prog_fd, fd, ProbeEvent { kind, event_alias })

@ -63,7 +63,6 @@ impl SkLookup {
pub fn attach<T: AsRawFd>(&mut self, netns: T) -> Result<SkLookupLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
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(

@ -81,7 +81,6 @@ impl SkMsg {
pub fn attach(&mut self, map: SockMapFd) -> Result<SkMsgLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let map_fd = map.as_raw_fd();
bpf_prog_attach(prog_fd, map_fd, BPF_SK_MSG_VERDICT).map_err(|(_, io_error)| {

@ -77,7 +77,6 @@ impl SkSkb {
pub fn attach(&mut self, map: SockMapFd) -> Result<SkSkbLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let map_fd = map.as_raw_fd();
let attach_type = match self.kind {

@ -61,7 +61,6 @@ impl SockOps {
pub fn attach<T: AsRawFd>(&mut self, cgroup: T) -> Result<SockOpsLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let cgroup_fd = cgroup.as_raw_fd();
bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS).map_err(|(_, io_error)| {

@ -4,7 +4,7 @@ use thiserror::Error;
use std::{
ffi::{CStr, CString},
io,
os::fd::{AsFd as _, AsRawFd as _},
os::fd::AsFd as _,
path::Path,
};
@ -155,7 +155,6 @@ impl SchedClassifier {
) -> Result<SchedClassifierLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let if_index = ifindex_from_ifname(interface)
.map_err(|io_error| TcError::NetlinkError { io_error })?;
let (priority, handle) = unsafe {

@ -1,9 +1,5 @@
//! Tracepoint programs.
use std::{
fs, io,
os::fd::{AsFd as _, AsRawFd as _},
path::Path,
};
use std::{fs, io, os::fd::AsFd as _, path::Path};
use thiserror::Error;
use crate::{
@ -84,7 +80,6 @@ impl TracePoint {
pub fn attach(&mut self, category: &str, name: &str) -> Result<TracePointLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let tracefs = find_tracefs_path()?;
let id = read_sys_fs_trace_point_id(tracefs, category, name)?;
let fd =

@ -20,7 +20,6 @@ pub(crate) fn attach_raw_tracepoint<T: Link + From<FdLink>>(
) -> Result<T::Id, ProgramError> {
let prog_fd = program_data.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let pfd =
bpf_raw_tracepoint_open(tp_name, prog_fd).map_err(|(_code, io_error)| SyscallError {
call: "bpf_raw_tracepoint_open",

@ -130,7 +130,6 @@ impl Xdp {
) -> Result<XdpLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
let if_index = if_index as RawFd;
if KernelVersion::current().unwrap() >= KernelVersion::new(5, 9, 0) {
@ -144,14 +143,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,7 +177,6 @@ impl Xdp {
pub fn attach_to_link(&mut self, link: XdpLink) -> Result<XdpLinkId, ProgramError> {
let prog_fd = self.fd()?;
let prog_fd = prog_fd.as_fd();
let prog_fd = prog_fd.as_raw_fd();
match link.into_inner() {
XdpLinkInner::FdLink(fd_link) => {
let link_fd = fd_link.fd;
@ -199,15 +197,20 @@ 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())
.map_err(|io_error| XdpError::NetlinkError { io_error })?;
netlink_set_xdp_fd(
if_index,
Some(prog_fd),
Some(old_prog_fd),
replace_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,
})))
}
@ -235,7 +238,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(())
}
}

@ -190,6 +190,7 @@ pub(crate) fn bpf_load_program(
if let Some(v) = aya_attr.attach_btf_id {
u.attach_btf_id = v;
}
bpf_prog_load(&mut attr)
}
@ -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) }
@ -678,7 +682,6 @@ pub(crate) fn is_perf_link_supported() -> bool {
if let Ok(fd) = bpf_prog_load(&mut attr) {
let fd = fd.as_fd();
let fd = fd.as_raw_fd();
matches!(
// Uses an invalid target FD so we get EBADF if supported.
bpf_link_create(fd, -1, bpf_attach_type::BPF_PERF_EVENT, None, 0),
@ -722,19 +725,17 @@ pub(crate) fn is_bpf_global_data_supported() -> bool {
pinned: false,
};
if let Ok(map_fd) = map_data.create("aya_global", None) {
insns[0].imm = map_fd;
let Ok(map_fd) = map_data.create("aya_global", None) else { return false };
let gpl = b"GPL\0";
u.license = gpl.as_ptr() as u64;
u.insn_cnt = insns.len() as u32;
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_SOCKET_FILTER as u32;
insns[0].imm = map_fd;
bpf_prog_load(&mut attr).is_ok()
} else {
false
}
let gpl = b"GPL\0";
u.license = gpl.as_ptr() as u64;
u.insn_cnt = insns.len() as u32;
u.insns = insns.as_ptr() as u64;
u.prog_type = bpf_prog_type::BPF_PROG_TYPE_SOCKET_FILTER as u32;
bpf_prog_load(&mut attr).is_ok()
}
pub(crate) fn is_bpf_cookie_supported() -> bool {

@ -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,

Loading…
Cancel
Save