From 25aa5ebcf23128b7b64d7cd38c01c604c4983e83 Mon Sep 17 00:00:00 2001 From: Ryan Alameddine Date: Fri, 14 Mar 2025 22:42:45 -0700 Subject: [PATCH 1/2] test: Add integration test for SkBuffContext::load_bytes Adds a test program `read_one` that triggers a verifier error when attempting to read one byte from the SkBuffContext. --- test/integration-ebpf/Cargo.toml | 4 ++++ test/integration-ebpf/src/socket_filter.rs | 20 +++++++++++++++++++ test/integration-test/src/lib.rs | 2 ++ test/integration-test/src/tests.rs | 1 + .../src/tests/socket_filter.rs | 13 ++++++++++++ 5 files changed, 40 insertions(+) create mode 100644 test/integration-ebpf/src/socket_filter.rs create mode 100644 test/integration-test/src/tests/socket_filter.rs diff --git a/test/integration-ebpf/Cargo.toml b/test/integration-ebpf/Cargo.toml index 8f3eb3de..1c2b04ea 100644 --- a/test/integration-ebpf/Cargo.toml +++ b/test/integration-ebpf/Cargo.toml @@ -91,3 +91,7 @@ path = "src/xdp_sec.rs" [[bin]] name = "uprobe_cookie" path = "src/uprobe_cookie.rs" + +[[bin]] +name = "socket_filter" +path = "src/socket_filter.rs" diff --git a/test/integration-ebpf/src/socket_filter.rs b/test/integration-ebpf/src/socket_filter.rs new file mode 100644 index 00000000..b9cf4cd8 --- /dev/null +++ b/test/integration-ebpf/src/socket_filter.rs @@ -0,0 +1,20 @@ +#![no_std] +#![no_main] + +use aya_ebpf::{macros::socket_filter, programs::SkBuffContext}; + + +#[socket_filter] +pub fn read_one(ctx: SkBuffContext) -> i64 { + // Read 1 byte + let mut dst = [0; 1]; + let _ = ctx.load_bytes(0, &mut dst); + + 0 +} + +#[cfg(not(test))] +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/test/integration-test/src/lib.rs b/test/integration-test/src/lib.rs index 90277bb4..50012891 100644 --- a/test/integration-test/src/lib.rs +++ b/test/integration-test/src/lib.rs @@ -32,6 +32,8 @@ pub const TEST: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/test") pub const TWO_PROGS: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/two_progs")); pub const XDP_SEC: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/xdp_sec")); pub const UPROBE_COOKIE: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/uprobe_cookie")); +pub const SOCKET_FILTER: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/socket_filter")); + #[cfg(test)] mod tests; #[cfg(test)] diff --git a/test/integration-test/src/tests.rs b/test/integration-test/src/tests.rs index 9ca83669..49cdc8ae 100644 --- a/test/integration-test/src/tests.rs +++ b/test/integration-test/src/tests.rs @@ -14,3 +14,4 @@ mod strncmp; mod tcx; mod uprobe_cookie; mod xdp; +mod socket_filter; \ No newline at end of file diff --git a/test/integration-test/src/tests/socket_filter.rs b/test/integration-test/src/tests/socket_filter.rs new file mode 100644 index 00000000..add7f98d --- /dev/null +++ b/test/integration-test/src/tests/socket_filter.rs @@ -0,0 +1,13 @@ +use aya::{programs::SocketFilter, Ebpf}; + +#[test] +fn socket_filter_load() { + let mut bpf = Ebpf::load(crate::SOCKET_FILTER).unwrap(); + let prog: &mut SocketFilter = bpf + .program_mut("read_one") + .unwrap() + .try_into() + .unwrap(); + prog.load().unwrap(); + +} \ No newline at end of file From 0b3c37bf73e7812ad46e0c46fecfda5cb55305ae Mon Sep 17 00:00:00 2001 From: Ryan Alameddine Date: Fri, 14 Mar 2025 23:35:45 -0700 Subject: [PATCH 2/2] fix(aya-ebpf): Add bounds check to SkBuff::load_bytes This resolves the verifier error triggered by the test case added in the previous commit --- ebpf/aya-ebpf/src/programs/sk_buff.rs | 6 +++++- test/integration-ebpf/src/socket_filter.rs | 3 +-- test/integration-test/src/tests.rs | 2 +- test/integration-test/src/tests/socket_filter.rs | 11 +++-------- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/ebpf/aya-ebpf/src/programs/sk_buff.rs b/ebpf/aya-ebpf/src/programs/sk_buff.rs index 46fd9cb6..cc46f932 100644 --- a/ebpf/aya-ebpf/src/programs/sk_buff.rs +++ b/ebpf/aya-ebpf/src/programs/sk_buff.rs @@ -10,7 +10,7 @@ use aya_ebpf_bindings::helpers::{ }; use aya_ebpf_cty::c_long; -use crate::{EbpfContext, bindings::__sk_buff}; +use crate::{EbpfContext, bindings::__sk_buff, check_bounds_signed}; pub struct SkBuff { pub skb: *mut __sk_buff, @@ -88,8 +88,12 @@ impl SkBuff { #[inline(always)] pub fn load_bytes(&self, offset: usize, dst: &mut [u8]) -> Result { let len = usize::try_from(self.len()).map_err(|core::num::TryFromIntError { .. }| -1)?; + // 0 byte reads will trip the verifier. We need to ensure that the valid range of values for len is at least 1. let len = len.checked_sub(offset).ok_or(-1)?; let len = len.min(dst.len()); + if !check_bounds_signed(len as i64, 1, dst.len() as i64) { + return Err(-1); + } let len_u32 = u32::try_from(len).map_err(|core::num::TryFromIntError { .. }| -1)?; let ret = unsafe { bpf_skb_load_bytes( diff --git a/test/integration-ebpf/src/socket_filter.rs b/test/integration-ebpf/src/socket_filter.rs index b9cf4cd8..783644cc 100644 --- a/test/integration-ebpf/src/socket_filter.rs +++ b/test/integration-ebpf/src/socket_filter.rs @@ -3,11 +3,10 @@ use aya_ebpf::{macros::socket_filter, programs::SkBuffContext}; - #[socket_filter] pub fn read_one(ctx: SkBuffContext) -> i64 { // Read 1 byte - let mut dst = [0; 1]; + let mut dst = [0; 2]; let _ = ctx.load_bytes(0, &mut dst); 0 diff --git a/test/integration-test/src/tests.rs b/test/integration-test/src/tests.rs index 49cdc8ae..edaa18da 100644 --- a/test/integration-test/src/tests.rs +++ b/test/integration-test/src/tests.rs @@ -10,8 +10,8 @@ mod rbpf; mod relocations; mod ring_buf; mod smoke; +mod socket_filter; mod strncmp; mod tcx; mod uprobe_cookie; mod xdp; -mod socket_filter; \ No newline at end of file diff --git a/test/integration-test/src/tests/socket_filter.rs b/test/integration-test/src/tests/socket_filter.rs index add7f98d..fdc0e80b 100644 --- a/test/integration-test/src/tests/socket_filter.rs +++ b/test/integration-test/src/tests/socket_filter.rs @@ -1,13 +1,8 @@ -use aya::{programs::SocketFilter, Ebpf}; +use aya::{Ebpf, programs::SocketFilter}; #[test] fn socket_filter_load() { let mut bpf = Ebpf::load(crate::SOCKET_FILTER).unwrap(); - let prog: &mut SocketFilter = bpf - .program_mut("read_one") - .unwrap() - .try_into() - .unwrap(); + let prog: &mut SocketFilter = bpf.program_mut("read_one").unwrap().try_into().unwrap(); prog.load().unwrap(); - -} \ No newline at end of file +}