From d3e6d9066fe432b226dc82688e19f1759b44082f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Medina?= Date: Sun, 28 May 2023 06:01:18 -0700 Subject: [PATCH] use OwnedFd for object BTF file this actually fixes a file descriptor leak since we were never closing it --- aya/src/bpf.rs | 142 ++++++++++++++++++++++++++++++--------------- aya/src/sys/bpf.rs | 6 +- 2 files changed, 98 insertions(+), 50 deletions(-) diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index 154ce6d6..8551f9db 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -4,9 +4,8 @@ use std::{ ffi::CString, fs, io, os::{ - fd::{AsFd, AsRawFd}, + fd::{AsFd, AsRawFd, OwnedFd}, raw::c_int, - unix::io::RawFd, }, path::{Path, PathBuf}, }; @@ -368,6 +367,9 @@ impl<'a> BpfLoader<'a> { None }; + // TODO (AM) + let raw_btf_fd = btf_fd.as_ref().map(|f| f.as_raw_fd()); + if let Some(btf) = &self.btf { obj.relocate_btf(btf)?; } @@ -400,7 +402,7 @@ impl<'a> BpfLoader<'a> { obj, fd: None, pinned: false, - btf_fd, + btf_fd: raw_btf_fd, }; match map.obj.pinning() { PinningType::ByName => { @@ -479,70 +481,90 @@ impl<'a> BpfLoader<'a> { let program = if self.extensions.contains(name.as_str()) { Program::Extension(Extension { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }) } else { match §ion { ProgramSection::KProbe { .. } => Program::KProbe(KProbe { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), kind: ProbeKind::KProbe, }), ProgramSection::KRetProbe { .. } => Program::KProbe(KProbe { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), kind: ProbeKind::KRetProbe, }), ProgramSection::UProbe { .. } => Program::UProbe(UProbe { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), kind: ProbeKind::UProbe, }), ProgramSection::URetProbe { .. } => Program::UProbe(UProbe { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), kind: ProbeKind::URetProbe, }), ProgramSection::TracePoint { .. } => Program::TracePoint(TracePoint { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::SocketFilter { .. } => { Program::SocketFilter(SocketFilter { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), }) } ProgramSection::Xdp { frags, .. } => { let mut data = - ProgramData::new(prog_name, obj, btf_fd, verifier_log_level); + ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level); if *frags { data.flags = BPF_F_XDP_HAS_FRAGS; } Program::Xdp(Xdp { data }) } ProgramSection::SkMsg { .. } => Program::SkMsg(SkMsg { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::CgroupSysctl { .. } => { Program::CgroupSysctl(CgroupSysctl { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), }) } ProgramSection::CgroupSockopt { attach_type, .. } => { Program::CgroupSockopt(CgroupSockopt { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), attach_type: *attach_type, }) } ProgramSection::SkSkbStreamParser { .. } => Program::SkSkb(SkSkb { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), kind: SkSkbKind::StreamParser, }), ProgramSection::SkSkbStreamVerdict { .. } => Program::SkSkb(SkSkb { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), kind: SkSkbKind::StreamVerdict, }), ProgramSection::SockOps { .. } => Program::SockOps(SockOps { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::SchedClassifier { .. } => { Program::SchedClassifier(SchedClassifier { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), name: unsafe { CString::from_vec_unchecked(Vec::from(name.clone())) .into_boxed_c_str() @@ -550,37 +572,47 @@ impl<'a> BpfLoader<'a> { }) } ProgramSection::CgroupSkb { .. } => Program::CgroupSkb(CgroupSkb { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), expected_attach_type: None, }), ProgramSection::CgroupSkbIngress { .. } => Program::CgroupSkb(CgroupSkb { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), expected_attach_type: Some(CgroupSkbAttachType::Ingress), }), ProgramSection::CgroupSkbEgress { .. } => Program::CgroupSkb(CgroupSkb { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), expected_attach_type: Some(CgroupSkbAttachType::Egress), }), ProgramSection::CgroupSockAddr { attach_type, .. } => { Program::CgroupSockAddr(CgroupSockAddr { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), attach_type: *attach_type, }) } ProgramSection::LircMode2 { .. } => Program::LircMode2(LircMode2 { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::PerfEvent { .. } => Program::PerfEvent(PerfEvent { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::RawTracePoint { .. } => { Program::RawTracePoint(RawTracePoint { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), }) } ProgramSection::Lsm { sleepable, .. } => { let mut data = - ProgramData::new(prog_name, obj, btf_fd, verifier_log_level); + ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level); if *sleepable { data.flags = BPF_F_SLEEPABLE; } @@ -588,30 +620,45 @@ impl<'a> BpfLoader<'a> { } ProgramSection::BtfTracePoint { .. } => { Program::BtfTracePoint(BtfTracePoint { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), }) } ProgramSection::FEntry { .. } => Program::FEntry(FEntry { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::FExit { .. } => Program::FExit(FExit { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::Extension { .. } => Program::Extension(Extension { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::SkLookup { .. } => Program::SkLookup(SkLookup { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new(prog_name, obj, raw_btf_fd, verifier_log_level), }), ProgramSection::CgroupSock { attach_type, .. } => { Program::CgroupSock(CgroupSock { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), attach_type: *attach_type, }) } ProgramSection::CgroupDevice { .. } => { Program::CgroupDevice(CgroupDevice { - data: ProgramData::new(prog_name, obj, btf_fd, verifier_log_level), + data: ProgramData::new( + prog_name, + obj, + raw_btf_fd, + verifier_log_level, + ), }) } } @@ -624,6 +671,7 @@ impl<'a> BpfLoader<'a> { Ok(Bpf { maps: maps?, programs, + _btf_fd: btf_fd, }) } } @@ -670,6 +718,7 @@ impl<'a> Default for BpfLoader<'a> { pub struct Bpf { maps: HashMap, programs: HashMap, + _btf_fd: Option, } impl Bpf { @@ -917,24 +966,21 @@ pub enum BpfError { ProgramError(#[from] ProgramError), } -fn load_btf(raw_btf: Vec) -> Result { +fn load_btf(raw_btf: Vec) -> Result { let mut logger = VerifierLog::new(); - let ret = retry_with_verifier_logs(10, &mut logger, |logger| { + retry_with_verifier_logs(10, &mut logger, |logger| { bpf_load_btf(raw_btf.as_slice(), logger) - }); - match ret { - Ok(fd) => Ok(fd as RawFd), - Err((_, io_error)) => { - logger.truncate(); - Err(BtfError::LoadError { - io_error, - verifier_log: logger - .as_c_str() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "[none]".to_owned()), - }) + }) + .map_err(|(_, io_error)| { + logger.truncate(); + BtfError::LoadError { + io_error, + verifier_log: logger + .as_c_str() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_else(|| "[none]".to_owned()), } - } + }) } /// Global data that can be exported to eBPF programs before they are loaded. diff --git a/aya/src/sys/bpf.rs b/aya/src/sys/bpf.rs index f31ce6fa..0bba20d3 100644 --- a/aya/src/sys/bpf.rs +++ b/aya/src/sys/bpf.rs @@ -563,7 +563,7 @@ pub(crate) fn bpf_raw_tracepoint_open( Ok(unsafe { OwnedFd::from_raw_fd(fd) }) } -pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> SysResult { +pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> Result { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_7 }; u.btf = raw_btf.as_ptr() as *const _ as u64; @@ -574,7 +574,9 @@ pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> SysResult { u.btf_log_buf = log_buf.as_mut_ptr() as u64; u.btf_log_size = log_buf.capacity() as u32; } - sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr) + let fd = sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr)? as RawFd; + // SAFETY: BPF_BTF_LOAD returns a new fd + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) } pub(crate) fn bpf_btf_get_fd_by_id(id: u32) -> io::Result {