aya: Use BorrowedFd when using the program fd in sys/bpf.rs

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.
pull/723/head
Andrés Medina 1 year ago committed by Tamir Duberstein
parent 4567c351fe
commit d2e74e562d
No known key found for this signature in database

@ -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) {

@ -88,7 +88,6 @@ impl Extension {
pub fn attach(&mut self) -> Result<ExtensionLinkId, ProgramError> {
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
@ -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},
};
@ -239,7 +239,12 @@ pub struct ProgAttachLink {
}
impl ProgAttachLink {
pub(crate) fn new(prog_fd: RawFd, target_fd: RawFd, attach_type: bpf_attach_type) -> Self {
pub(crate) fn new(
prog_fd: BorrowedFd<'_>,
target_fd: RawFd,
attach_type: bpf_attach_type,
) -> Self {
let prog_fd = prog_fd.as_raw_fd();
Self {
prog_fd,
target_fd: unsafe { dup(target_fd) },

@ -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,7 +131,8 @@ pub struct LircLink {
}
impl LircLink {
pub(crate) fn new(prog_fd: RawFd, target_fd: RawFd) -> Self {
pub(crate) fn new(prog_fd: BorrowedFd<'_>, target_fd: RawFd) -> Self {
let prog_fd = prog_fd.as_raw_fd();
Self {
prog_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: 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)),

@ -5,7 +5,7 @@ use std::{
fmt::Write as _,
fs::{self, OpenOptions},
io::{self, Write},
os::fd::{AsFd as _, AsRawFd as _, OwnedFd},
os::fd::{AsFd as _, OwnedFd},
path::{Path, PathBuf},
process,
sync::atomic::{AtomicUsize, Ordering},
@ -115,7 +115,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,
};
@ -154,7 +154,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 name = self.data.name.as_deref().unwrap_or_default();

@ -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.as_ref())?;
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",

@ -128,7 +128,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) {
@ -147,9 +146,10 @@ 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 })?;
let prog_fd = prog_fd.as_raw_fd();
self.data
.links
.insert(XdpLink::new(XdpLinkInner::NlLink(NlLink {
@ -181,7 +181,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;
@ -202,10 +201,16 @@ 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 })?;
}
let prog_fd = prog_fd.as_raw_fd();
self.data
.links
.insert(XdpLink::new(XdpLinkInner::NlLink(NlLink {
@ -238,7 +243,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(())
}
}

@ -367,7 +367,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 +375,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 +390,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 +409,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 +547,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 +681,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),

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