diff --git a/bpf/aya-bpf/src/helpers.rs b/bpf/aya-bpf/src/helpers.rs index b740f4b9..69ab3ccc 100644 --- a/bpf/aya-bpf/src/helpers.rs +++ b/bpf/aya-bpf/src/helpers.rs @@ -13,7 +13,10 @@ pub use aya_bpf_bindings::helpers as gen; #[doc(hidden)] pub use gen::*; -use crate::cty::{c_char, c_long, c_void}; +use crate::{ + check_bounds_signed, + cty::{c_char, c_long, c_void}, +}; /// Read bytes stored at `src` and store them as a `T`. /// @@ -419,19 +422,23 @@ pub unsafe fn bpf_probe_read_user_str_bytes( dest.len() as u32, src as *const c_void, ); - if len <= 0 { - return Err(-1); - } - let len = len as usize; - if len >= dest.len() { - // this can never happen, it's needed to tell the verifier that len is - // bounded + read_str_bytes(len, dest) +} + +fn read_str_bytes(len: i64, dest: &mut [u8]) -> Result<&[u8], c_long> { + // The lower bound is 0, since it's what is returned for b"\0". See the + // bpf_probe_read_user_[user|kernel]_bytes_empty integration tests. The upper bound + // check is not needed since the helper truncates, but the verifier doesn't + // know that so we show it the upper bound. + if !check_bounds_signed(len, 0, dest.len() as i64) { return Err(-1); } - // len includes NULL byte - Ok(&dest[..len - 1]) + // len includes the NULL terminator but not for b"\0" for which the kernel + // returns len=0. So we do a saturating sub and for b"\0" we return the + // empty slice, for all other cases we omit the terminator. + Ok(&dest[..(len as usize).saturating_sub(1)]) } /// Read a null-terminated string from _kernel space_ stored at `src` into `dest`. @@ -572,19 +579,8 @@ pub unsafe fn bpf_probe_read_kernel_str_bytes( dest.len() as u32, src as *const c_void, ); - if len <= 0 { - return Err(-1); - } - - let len = len as usize; - if len >= dest.len() { - // this can never happen, it's needed to tell the verifier that len is - // bounded - return Err(-1); - } - // len includes NULL byte - Ok(&dest[..len - 1]) + read_str_bytes(len, dest) } /// Write bytes to the _user space_ pointer `src` and store them as a `T`. diff --git a/bpf/aya-bpf/src/lib.rs b/bpf/aya-bpf/src/lib.rs index 57e0e372..c0edd49a 100644 --- a/bpf/aya-bpf/src/lib.rs +++ b/bpf/aya-bpf/src/lib.rs @@ -4,12 +4,12 @@ //! //! Aya-bpf is an eBPF library built with a focus on operability and developer experience. //! It is the kernel-space counterpart of [Aya](https://docs.rs/aya) - #![doc( html_logo_url = "https://aya-rs.dev/assets/images/crabby.svg", html_favicon_url = "https://aya-rs.dev/assets/images/crabby.svg" )] #![cfg_attr(unstable, feature(never_type))] +#![cfg_attr(target_arch = "bpf", feature(asm_experimental_arch))] #![allow(clippy::missing_safety_doc)] #![no_std] @@ -72,3 +72,31 @@ pub unsafe extern "C" fn memcpy(dest: *mut u8, src: *mut u8, n: usize) { *((dest_base + i) as *mut u8) = *((src_base + i) as *mut u8); } } + +/// Check if a value is within a range, using conditional forms compatible with +/// the verifier. +#[inline(always)] +pub fn check_bounds_signed(value: i64, lower: i64, upper: i64) -> bool { + #[cfg(target_arch = "bpf")] + unsafe { + let mut in_bounds = 0u64; + core::arch::asm!( + "if {value} s< {lower} goto +2", + "if {value} s> {upper} goto +1", + "{i} = 1", + i = inout(reg) in_bounds, + lower = in(reg) lower, + upper = in(reg) upper, + value = in(reg) value, + ); + in_bounds == 1 + } + // We only need this for doc tests which are compiled for the host target + #[cfg(not(target_arch = "bpf"))] + { + let _ = value; + let _ = lower; + let _ = upper; + unimplemented!() + } +} diff --git a/test/integration-ebpf/Cargo.toml b/test/integration-ebpf/Cargo.toml index 0683143a..73edf5fd 100644 --- a/test/integration-ebpf/Cargo.toml +++ b/test/integration-ebpf/Cargo.toml @@ -31,3 +31,7 @@ path = "src/test.rs" [[bin]] name = "relocations" path = "src/relocations.rs" + +[[bin]] +name = "bpf_probe_read" +path = "src/bpf_probe_read.rs" \ No newline at end of file diff --git a/test/integration-ebpf/src/bpf_probe_read.rs b/test/integration-ebpf/src/bpf_probe_read.rs new file mode 100644 index 00000000..013c8b96 --- /dev/null +++ b/test/integration-ebpf/src/bpf_probe_read.rs @@ -0,0 +1,91 @@ +#![no_std] +#![no_main] + +use aya_bpf::{ + helpers::{bpf_probe_read_kernel_str_bytes, bpf_probe_read_user_str_bytes}, + macros::{map, uprobe}, + maps::Array, + programs::ProbeContext, +}; + +const RESULT_BUF_LEN: usize = 1024; + +macro_rules! read_str_bytes { + ($fun:ident, $ptr:expr, $len:expr $(,)?) => { + let r = unsafe { + let Some(ptr) = RESULT.get_ptr_mut(0) else { + return; + }; + &mut *ptr + }; + + let s = unsafe { + // $len comes from ctx.arg(1) so it's dynamic and the verifier + // doesn't see any bounds. We do $len.min(RESULT_BUF_LEN) here to + // ensure that the verifier can see the upper bound, or you get: + // + // 18: (79) r7 = *(u64 *)(r7 +8) ; R7_w=scalar() + // [snip] + // 27: (bf) r2 = r7 ; + // R2_w=scalar(id=2,umax=9223372036854775807,var_off=(0x0; 0x7fffffffffffffff)) [snip] + // 28: (85) call bpf_probe_read_user_str#114 + // R2 unbounded memory access, use 'var &= const' or 'if (var < const)' + match $fun($ptr, &mut r.buf[..$len.min(RESULT_BUF_LEN)]) { + Ok(s) => s, + Err(_) => { + r.did_error = 1; + return; + } + } + }; + r.len = s.len(); + }; +} + +#[repr(C)] +struct TestResult { + did_error: u64, + len: usize, + buf: [u8; RESULT_BUF_LEN], +} + +#[map] +static RESULT: Array = Array::with_max_entries(1, 0); + +#[map] +static KERNEL_BUFFER: Array<[u8; 1024]> = Array::with_max_entries(1, 0); + +#[uprobe] +pub fn test_bpf_probe_read_user_str_bytes(ctx: ProbeContext) { + read_str_bytes!( + bpf_probe_read_user_str_bytes, + match ctx.arg::<*const u8>(0) { + Some(p) => p, + _ => return, + }, + match ctx.arg::(1) { + Some(p) => p, + _ => return, + }, + ); +} + +#[uprobe] +pub fn test_bpf_probe_read_kernel_str_bytes(ctx: ProbeContext) { + read_str_bytes!( + bpf_probe_read_kernel_str_bytes, + match KERNEL_BUFFER.get_ptr(0) { + Some(p) => p as *const u8, + _ => return, + }, + match ctx.arg::(0) { + Some(p) => p, + _ => return, + }, + ); +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + unsafe { core::hint::unreachable_unchecked() } +} diff --git a/test/integration-test/src/tests/bpf_probe_read.rs b/test/integration-test/src/tests/bpf_probe_read.rs new file mode 100644 index 00000000..175ed114 --- /dev/null +++ b/test/integration-test/src/tests/bpf_probe_read.rs @@ -0,0 +1,140 @@ +use std::process::exit; + +use aya::{ + include_bytes_aligned, + maps::Array, + programs::{ProgramError, UProbe}, + Bpf, +}; +use integration_test_macros::integration_test; + +const RESULT_BUF_LEN: usize = 1024; + +#[derive(Copy, Clone)] +#[repr(C)] +struct TestResult { + did_error: u64, + len: usize, + buf: [u8; RESULT_BUF_LEN], +} + +unsafe impl aya::Pod for TestResult {} + +#[integration_test] +fn bpf_probe_read_user_str_bytes() { + let bpf = set_user_buffer(b"foo\0", RESULT_BUF_LEN); + assert_eq!(result_bytes(&bpf), b"foo"); +} + +#[integration_test] +fn bpf_probe_read_user_str_bytes_truncate() { + let s = vec![b'a'; RESULT_BUF_LEN]; + let bpf = set_user_buffer(&s, RESULT_BUF_LEN); + // The kernel truncates the string and the last byte is the null terminator + assert_eq!(result_bytes(&bpf), &s[..RESULT_BUF_LEN - 1]); +} + +#[integration_test] +fn bpf_probe_read_user_str_bytes_empty_string() { + let bpf = set_user_buffer(b"\0", RESULT_BUF_LEN); + assert_eq!(result_bytes(&bpf), b""); +} + +#[integration_test] +fn bpf_probe_read_user_str_bytes_empty_dest() { + let bpf = set_user_buffer(b"foo\0", 0); + assert_eq!(result_bytes(&bpf), b""); +} + +#[integration_test] +fn bpf_probe_read_kernel_str_bytes() { + let bpf = set_kernel_buffer(b"foo\0", RESULT_BUF_LEN); + assert_eq!(result_bytes(&bpf), b"foo"); +} + +#[integration_test] +fn bpf_probe_read_kernel_str_bytes_truncate() { + let s = vec![b'a'; RESULT_BUF_LEN]; + let bpf = set_kernel_buffer(&s, RESULT_BUF_LEN); + // The kernel truncates the string and the last byte is the null terminator + assert_eq!(result_bytes(&bpf), &s[..RESULT_BUF_LEN - 1]); +} + +#[integration_test] +fn bpf_probe_read_kernel_str_bytes_empty_string() { + let bpf = set_kernel_buffer(b"\0", RESULT_BUF_LEN); + assert_eq!(result_bytes(&bpf), b""); +} + +#[integration_test] +fn bpf_probe_read_kernel_str_bytes_empty_dest() { + let bpf = set_kernel_buffer(b"foo\0", 0); + assert_eq!(result_bytes(&bpf), b""); +} + +fn set_user_buffer(bytes: &[u8], dest_len: usize) -> Bpf { + let bpf = load_and_attach_uprobe( + "test_bpf_probe_read_user_str_bytes", + "trigger_bpf_probe_read_user", + include_bytes_aligned!("../../../../target/bpfel-unknown-none/release/bpf_probe_read"), + ); + trigger_bpf_probe_read_user(bytes.as_ptr(), dest_len); + bpf +} + +fn set_kernel_buffer(bytes: &[u8], dest_len: usize) -> Bpf { + let mut bpf = load_and_attach_uprobe( + "test_bpf_probe_read_kernel_str_bytes", + "trigger_bpf_probe_read_kernel", + include_bytes_aligned!("../../../../target/bpfel-unknown-none/release/bpf_probe_read"), + ); + set_kernel_buffer_element(&mut bpf, bytes); + trigger_bpf_probe_read_kernel(dest_len); + bpf +} + +fn set_kernel_buffer_element(bpf: &mut Bpf, bytes: &[u8]) { + let mut bytes = bytes.to_vec(); + bytes.resize(1024, 0xFF); + let bytes: [u8; 1024] = bytes.try_into().unwrap(); + let mut m = Array::<_, [u8; 1024]>::try_from(bpf.map_mut("KERNEL_BUFFER").unwrap()).unwrap(); + m.set(0, bytes, 0).unwrap(); +} + +fn result_bytes(bpf: &Bpf) -> Vec { + let m = Array::<_, TestResult>::try_from(bpf.map("RESULT").unwrap()).unwrap(); + let result = m.get(&0, 0).unwrap(); + assert!(result.did_error == 0); + // assert that the buffer is always null terminated + assert_eq!(result.buf[result.len], 0); + result.buf[..result.len].to_vec() +} + +fn load_and_attach_uprobe(prog_name: &str, func_name: &str, bytes: &[u8]) -> Bpf { + let mut bpf = Bpf::load(bytes).unwrap(); + + let prog: &mut UProbe = bpf.program_mut(prog_name).unwrap().try_into().unwrap(); + if let Err(ProgramError::LoadError { + io_error, + verifier_log, + }) = prog.load() + { + println!( + "Failed to load program `{prog_name}`: {io_error}. Verifier log:\n{verifier_log:#}" + ); + exit(1); + }; + + prog.attach(Some(func_name), 0, "/proc/self/exe", None) + .unwrap(); + + bpf +} + +#[no_mangle] +#[inline(never)] +pub extern "C" fn trigger_bpf_probe_read_user(_string: *const u8, _len: usize) {} + +#[no_mangle] +#[inline(never)] +pub extern "C" fn trigger_bpf_probe_read_kernel(_len: usize) {} diff --git a/test/integration-test/src/tests/mod.rs b/test/integration-test/src/tests/mod.rs index c26ca5a4..93e6aaa3 100644 --- a/test/integration-test/src/tests/mod.rs +++ b/test/integration-test/src/tests/mod.rs @@ -4,6 +4,7 @@ use libc::{uname, utsname}; use regex::Regex; use std::{ffi::CStr, mem}; +pub mod bpf_probe_read; pub mod btf_relocations; pub mod elf; pub mod load;