From 6b94b2080dc4c122954beea814b2a1a4569e9aa3 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sun, 9 Jul 2023 14:47:49 -0400 Subject: [PATCH] Hide details of VerifierLog This type is really only used by one function. --- aya-obj/src/btf/btf.rs | 2 +- aya/src/bpf.rs | 22 +++++---------- aya/src/programs/mod.rs | 22 +++++---------- aya/src/sys/bpf.rs | 62 ++++++++++++++++++++++------------------- aya/src/util.rs | 53 +---------------------------------- 5 files changed, 49 insertions(+), 112 deletions(-) diff --git a/aya-obj/src/btf/btf.rs b/aya-obj/src/btf/btf.rs index a9049b1e..295fb55e 100644 --- a/aya-obj/src/btf/btf.rs +++ b/aya-obj/src/btf/btf.rs @@ -135,7 +135,7 @@ pub enum BtfError { #[source] io_error: std::io::Error, /// The error log produced by the kernel verifier. - verifier_log: String, + verifier_log: Cow<'static, str>, }, /// offset not found for symbol diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index 74eb5317..ce4273d4 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -39,7 +39,7 @@ use crate::{ is_btf_supported, is_btf_type_tag_supported, is_perf_link_supported, is_probe_read_kernel_supported, is_prog_name_supported, retry_with_verifier_logs, }, - util::{bytes_of, bytes_of_slice, possible_cpus, VerifierLog, POSSIBLE_CPUS}, + util::{bytes_of, bytes_of_slice, possible_cpus, POSSIBLE_CPUS}, }; pub(crate) const BPF_OBJ_NAME_LEN: usize = 16; @@ -907,22 +907,14 @@ pub enum BpfError { } fn load_btf(raw_btf: Vec) -> Result { - let mut logger = VerifierLog::new(); - let ret = retry_with_verifier_logs(10, &mut logger, |logger| { - bpf_load_btf(raw_btf.as_slice(), logger) - }); + let (ret, verifier_log) = + retry_with_verifier_logs(10, |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()), - }) - } + Err((_, io_error)) => Err(BtfError::LoadError { + io_error, + verifier_log, + }), } } diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index 5aa6b156..793c94e6 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -67,6 +67,7 @@ pub mod xdp; use libc::ENOSPC; use procfs::KernelVersion; use std::{ + borrow::Cow, ffi::CString, io, os::unix::io::{AsRawFd, RawFd}, @@ -113,7 +114,6 @@ use crate::{ bpf_prog_get_fd_by_id, bpf_prog_get_info_by_fd, bpf_prog_get_next_id, bpf_prog_query, retry_with_verifier_logs, BpfLoadProgramAttrs, }, - util::VerifierLog, }; /// Error type returned when working with programs. @@ -142,7 +142,7 @@ pub enum ProgramError { #[source] io_error: io::Error, /// The error log produced by the kernel verifier. - verifier_log: String, + verifier_log: Cow<'static, str>, }, /// A syscall failed. @@ -583,8 +583,6 @@ fn load_program( (u32::from(major) << 16) + (u32::from(minor) << 8) + u32::from(patch) }); - let mut logger = VerifierLog::new(); - let prog_name = if let Some(name) = &data.name { let mut name = name.clone(); if name.len() > 15 { @@ -616,7 +614,7 @@ fn load_program( }; let verifier_log_level = data.verifier_log_level; - let ret = retry_with_verifier_logs(10, &mut logger, |logger| { + let (ret, verifier_log) = retry_with_verifier_logs(10, |logger| { bpf_load_program(&attr, logger, verifier_log_level) }); @@ -625,16 +623,10 @@ fn load_program( *fd = Some(prog_fd as RawFd); Ok(()) } - Err((_, io_error)) => { - logger.truncate(); - return Err(ProgramError::LoadError { - io_error, - verifier_log: logger - .as_c_str() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "[none]".to_owned()), - }); - } + Err((_, io_error)) => Err(ProgramError::LoadError { + io_error, + verifier_log, + }), } } diff --git a/aya/src/sys/bpf.rs b/aya/src/sys/bpf.rs index 542896cd..b5fe2d64 100644 --- a/aya/src/sys/bpf.rs +++ b/aya/src/sys/bpf.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Cow, cmp::{self, min}, ffi::{CStr, CString}, io, @@ -29,7 +30,6 @@ use crate::{ copy_instructions, }, sys::{syscall, SysResult, Syscall}, - util::VerifierLog, Btf, Pod, BPF_OBJ_NAME_LEN, }; @@ -129,7 +129,7 @@ pub(crate) struct BpfLoadProgramAttrs<'a> { pub(crate) fn bpf_load_program( aya_attr: &BpfLoadProgramAttrs, - logger: &mut VerifierLog, + log_buf: &mut [u8], verifier_log_level: u32, ) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; @@ -174,11 +174,10 @@ pub(crate) fn bpf_load_program( u.func_info_rec_size = aya_attr.func_info_rec_size as u32; } } - let log_buf = logger.buf(); - if log_buf.capacity() > 0 { + if !log_buf.is_empty() { u.log_level = verifier_log_level; u.log_buf = log_buf.as_mut_ptr() as u64; - u.log_size = log_buf.capacity() as u32; + u.log_size = log_buf.len() as u32; } if let Some(v) = aya_attr.attach_btf_obj_fd { u.__bindgen_anon_1.attach_btf_obj_fd = v; @@ -549,16 +548,15 @@ pub(crate) fn bpf_raw_tracepoint_open(name: Option<&CStr>, prog_fd: RawFd) -> Sy sys_bpf(bpf_cmd::BPF_RAW_TRACEPOINT_OPEN, &attr) } -pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> SysResult { +pub(crate) fn bpf_load_btf(raw_btf: &[u8], log_buf: &mut [u8]) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_7 }; u.btf = raw_btf.as_ptr() as *const _ as u64; u.btf_size = mem::size_of_val(raw_btf) as u32; - let log_buf = log.buf(); - if log_buf.capacity() > 0 { + if !log_buf.is_empty() { u.btf_log_level = 1; u.btf_log_buf = log_buf.as_mut_ptr() as u64; - u.btf_log_size = log_buf.capacity() as u32; + u.btf_log_size = log_buf.len() as u32; } sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr) } @@ -993,35 +991,41 @@ pub(crate) fn bpf_prog_get_next_id(id: u32) -> Result, (c_long, io:: pub(crate) fn retry_with_verifier_logs( max_retries: usize, - log: &mut VerifierLog, f: F, -) -> SysResult +) -> (SysResult, Cow<'static, str>) where - F: Fn(&mut VerifierLog) -> SysResult, + F: Fn(&mut [u8]) -> SysResult, { - // 1. Try the syscall - let ret = f(log); - if ret.is_ok() { - return ret; - } + const MIN_LOG_BUF_SIZE: usize = 1024 * 10; + const MAX_LOG_BUF_SIZE: usize = (std::u32::MAX >> 8) as usize; - // 2. Grow the log buffer so we can capture verifier output - // Retry this up to max_retries times - log.grow(); + let mut log_buf = Vec::new(); let mut retries = 0; - loop { - let ret = f(log); - match ret { - Err((v, io_error)) if retries == 0 || io_error.raw_os_error() == Some(ENOSPC) => { - if retries == max_retries { - return Err((v, io_error)); + let ret = f(log_buf.as_mut_slice()); + if retries != max_retries { + if let Err((_, io_error)) = &ret { + if retries == 0 || io_error.raw_os_error() == Some(ENOSPC) { + let len = (log_buf.capacity() * 10).clamp(MIN_LOG_BUF_SIZE, MAX_LOG_BUF_SIZE); + log_buf.resize(len, 0); + if let Some(first) = log_buf.first_mut() { + *first = 0; + } + retries += 1; + continue; } - retries += 1; - log.grow(); } - r => return r, } + if let Some(pos) = log_buf.iter().position(|b| *b == 0) { + log_buf.truncate(pos); + } + let log_buf = if log_buf.is_empty() { + "none".into() + } else { + String::from_utf8(log_buf).unwrap().into() + }; + + break (ret, log_buf); } } diff --git a/aya/src/util.rs b/aya/src/util.rs index e3106046..b0bb3781 100644 --- a/aya/src/util.rs +++ b/aya/src/util.rs @@ -1,7 +1,7 @@ //! Utility functions. use std::{ collections::BTreeMap, - ffi::{CStr, CString}, + ffi::CString, fs::{self, File}, io::{self, BufReader}, mem, slice, @@ -200,57 +200,6 @@ pub(crate) fn bytes_of_slice(val: &[T]) -> &[u8] { unsafe { slice::from_raw_parts(val.as_ptr().cast(), size) } } -const MIN_LOG_BUF_SIZE: usize = 1024 * 10; -const MAX_LOG_BUF_SIZE: usize = (std::u32::MAX >> 8) as usize; - -pub(crate) struct VerifierLog { - buf: Vec, -} - -impl VerifierLog { - pub(crate) fn new() -> VerifierLog { - VerifierLog { buf: Vec::new() } - } - - pub(crate) fn buf(&mut self) -> &mut Vec { - &mut self.buf - } - - pub(crate) fn grow(&mut self) { - let len = (self.buf.capacity() * 10).clamp(MIN_LOG_BUF_SIZE, MAX_LOG_BUF_SIZE); - self.buf.resize(len, 0); - self.reset(); - } - - pub(crate) fn reset(&mut self) { - if !self.buf.is_empty() { - self.buf[0] = 0; - } - } - - pub(crate) fn truncate(&mut self) { - if self.buf.is_empty() { - return; - } - - let pos = self - .buf - .iter() - .position(|b| *b == 0) - .unwrap_or(self.buf.len() - 1); - self.buf[pos] = 0; - self.buf.truncate(pos + 1); - } - - pub(crate) fn as_c_str(&self) -> Option<&CStr> { - if self.buf.is_empty() { - None - } else { - Some(CStr::from_bytes_with_nul(&self.buf).unwrap()) - } - } -} - #[cfg(test)] mod tests { use super::*;