From 5d8b279265bd2715b83cbed871697bbc763a00a9 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Fri, 14 Jan 2022 18:39:44 +0000 Subject: [PATCH] aya: Fix BTF verifier output Currently errors can occur if the verifier output is > buffer as we get ENOMEM. We should only provide a log_buf if initial load failed, then retry up to 10 times to get full verifier output. To DRY this logic it has been moved to a function so its shared with program loading Signed-off-by: Dave Tucker one verifier loop to rule them all Signed-off-by: Dave Tucker --- aya/src/bpf.rs | 39 +++++++++++-------- aya/src/programs/mod.rs | 83 +++++++++++++++++------------------------ aya/src/sys/bpf.rs | 53 +++++++++++++++++++++----- 3 files changed, 100 insertions(+), 75 deletions(-) diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index 4f417fc0..b0c18161 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -30,6 +30,7 @@ use crate::{ bpf_load_btf, bpf_map_freeze, bpf_map_update_elem_ptr, is_btf_datasec_supported, is_btf_decl_tag_supported, is_btf_float_supported, is_btf_func_global_supported, is_btf_func_supported, is_btf_supported, is_btf_type_tag_supported, is_prog_name_supported, + retry_with_verifier_logs, }, util::{bytes_of, possible_cpus, VerifierLog, POSSIBLE_CPUS}, }; @@ -314,22 +315,8 @@ impl<'a> BpfLoader<'a> { // load btf to the kernel let raw_btf = btf.to_bytes(); - let mut log_buf = VerifierLog::new(); - log_buf.grow(); - let ret = bpf_load_btf(raw_btf.as_slice(), &mut log_buf); - match ret { - Ok(fd) => Some(fd), - Err(io_error) => { - log_buf.truncate(); - return Err(BpfError::BtfError(BtfError::LoadError { - io_error, - verifier_log: log_buf - .as_c_str() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "[none]".to_owned()), - })); - } - } + let fd = load_btf(raw_btf)?; + Some(fd) } else { None } @@ -768,3 +755,23 @@ pub enum BpfError { /// A program error ProgramError(#[from] ProgramError), } + +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) + }); + 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()), + }) + } + } +} diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index 25cebf66..d2b6ac93 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -97,7 +97,7 @@ use crate::{ obj::{self, btf::BtfError, Function, KernelVersion}, sys::{ bpf_get_object, bpf_load_program, bpf_obj_get_info_by_fd, bpf_pin_object, bpf_prog_detach, - bpf_prog_get_fd_by_id, bpf_prog_query, BpfLoadProgramAttrs, + bpf_prog_get_fd_by_id, bpf_prog_query, retry_with_verifier_logs, BpfLoadProgramAttrs, }, util::VerifierLog, }; @@ -412,9 +412,7 @@ fn load_program(prog_type: bpf_prog_type, data: &mut ProgramData) -> Result<(), _ => (*kernel_version).into(), }; - let mut log_buf = VerifierLog::new(); - let mut retries = 0; - let mut ret; + let mut logger = VerifierLog::new(); let prog_name = if let Some(name) = &data.name { let mut name = name.clone(); @@ -428,53 +426,40 @@ fn load_program(prog_type: bpf_prog_type, data: &mut ProgramData) -> Result<(), None }; - loop { - let attr = BpfLoadProgramAttrs { - name: prog_name.clone(), - ty: prog_type, - insns: instructions, - license, - kernel_version: target_kernel_version, - expected_attach_type: data.expected_attach_type, - prog_btf_fd: data.btf_fd, - attach_btf_obj_fd: data.attach_btf_obj_fd, - attach_btf_id: data.attach_btf_id, - attach_prog_fd: data.attach_prog_fd, - log: &mut log_buf, - func_info_rec_size: *func_info_rec_size, - func_info: func_info.clone(), - line_info_rec_size: *line_info_rec_size, - line_info: line_info.clone(), - }; - ret = bpf_load_program(attr); - match &ret { - Ok(prog_fd) => { - *fd = Some(*prog_fd as RawFd); - return Ok(()); - } - Err((_, io_error)) if retries == 0 || io_error.raw_os_error() == Some(ENOSPC) => { - if retries == 10 { - break; - } - retries += 1; - log_buf.grow(); - } - Err(_) => break, - }; - } + let attr = BpfLoadProgramAttrs { + name: prog_name, + ty: prog_type, + insns: instructions, + license, + kernel_version: target_kernel_version, + expected_attach_type: data.expected_attach_type, + prog_btf_fd: data.btf_fd, + attach_btf_obj_fd: data.attach_btf_obj_fd, + attach_btf_id: data.attach_btf_id, + attach_prog_fd: data.attach_prog_fd, + func_info_rec_size: *func_info_rec_size, + func_info: func_info.clone(), + line_info_rec_size: *line_info_rec_size, + line_info: line_info.clone(), + }; + let ret = retry_with_verifier_logs(10, &mut logger, |logger| bpf_load_program(&attr, logger)); - if let Err((_, io_error)) = ret { - log_buf.truncate(); - return Err(ProgramError::LoadError { - io_error, - verifier_log: log_buf - .as_c_str() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "[none]".to_owned()), - }); + match ret { + Ok(prog_fd) => { + *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()), + }); + } } - - Ok(()) } pub(crate) fn query( diff --git a/aya/src/sys/bpf.rs b/aya/src/sys/bpf.rs index 3f198360..69d03761 100644 --- a/aya/src/sys/bpf.rs +++ b/aya/src/sys/bpf.rs @@ -3,7 +3,7 @@ use crate::{ obj::{btf::BtfType, copy_instructions}, Btf, }; -use libc::{c_char, c_long, close, ENOENT}; +use libc::{c_char, c_long, close, ENOENT, ENOSPC}; use std::{ cmp::{self, min}, @@ -78,19 +78,21 @@ pub(crate) struct BpfLoadProgramAttrs<'a> { pub(crate) attach_btf_obj_fd: Option, pub(crate) attach_btf_id: Option, pub(crate) attach_prog_fd: Option, - pub(crate) log: &'a mut VerifierLog, pub(crate) func_info_rec_size: usize, pub(crate) func_info: FuncSecInfo, pub(crate) line_info_rec_size: usize, pub(crate) line_info: LineSecInfo, } -pub(crate) fn bpf_load_program(aya_attr: BpfLoadProgramAttrs) -> SysResult { +pub(crate) fn bpf_load_program( + aya_attr: &BpfLoadProgramAttrs, + logger: &mut VerifierLog, +) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_3 }; - if let Some(prog_name) = aya_attr.name { + if let Some(prog_name) = &aya_attr.name { let mut name: [c_char; 16] = [0; 16]; let name_bytes = prog_name.to_bytes(); let len = min(name.len(), name_bytes.len()); @@ -127,7 +129,7 @@ pub(crate) fn bpf_load_program(aya_attr: BpfLoadProgramAttrs) -> SysResult { u.func_info_rec_size = aya_attr.func_info_rec_size as u32; } } - let log_buf = aya_attr.log.buf(); + let log_buf = logger.buf(); if log_buf.capacity() > 0 { u.log_level = 7; u.log_buf = log_buf.as_mut_ptr() as u64; @@ -443,7 +445,7 @@ 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) -> Result { +pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> 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; @@ -454,10 +456,7 @@ pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> Result Ok(v as RawFd), - Err((_, err)) => Err(err), - } + sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr) } pub(crate) fn bpf_btf_get_fd_by_id(id: u32) -> Result { @@ -733,3 +732,37 @@ pub(crate) fn is_btf_type_tag_supported() -> bool { pub fn sys_bpf(cmd: bpf_cmd, attr: &bpf_attr) -> SysResult { syscall(Syscall::Bpf { cmd, attr }) } + +pub(crate) fn retry_with_verifier_logs( + max_retries: usize, + log: &mut VerifierLog, + f: F, +) -> SysResult +where + F: Fn(&mut VerifierLog) -> SysResult, +{ + // 1. Try the syscall + let ret = f(log); + if ret.is_ok() { + return ret; + } + + // 2. Grow the log buffer so we can capture verifier output + // Retry this up to max_retries times + log.grow(); + 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)); + } + retries += 1; + log.grow(); + } + r => return r, + } + } +}