From 77f70308f82a2559382d3ff1941d4847182ec6e6 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. Signed-off-by: benaryorg --- test/integration-ebpf/Cargo.toml | 4 ++++ test/integration-ebpf/src/socket_filter.rs | 17 +++++++++++++++++ test/integration-test/src/lib.rs | 1 + test/integration-test/src/tests.rs | 1 + .../integration-test/src/tests/socket_filter.rs | 8 ++++++++ 5 files changed, 31 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 864ed82fb..72a339162 100644 --- a/test/integration-ebpf/Cargo.toml +++ b/test/integration-ebpf/Cargo.toml @@ -135,3 +135,7 @@ path = "src/printk_test.rs" [[bin]] name = "prog_array" path = "src/prog_array.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 000000000..f3ad7d354 --- /dev/null +++ b/test/integration-ebpf/src/socket_filter.rs @@ -0,0 +1,17 @@ +#![no_std] +#![no_main] +#![expect(unused_crate_dependencies, reason = "used in other bins")] + +use aya_ebpf::{macros::socket_filter, programs::SkBuffContext}; + +#[cfg(not(test))] +extern crate ebpf_panic; + +#[socket_filter] +fn read_one(ctx: SkBuffContext) -> i64 { + // Read 1 byte + let mut dst = [0; 2]; + let _result: Result<_, _> = ctx.load_bytes(0, &mut dst); + + 0 +} diff --git a/test/integration-test/src/lib.rs b/test/integration-test/src/lib.rs index a4d9ab1eb..d2d1471e9 100644 --- a/test/integration-test/src/lib.rs +++ b/test/integration-test/src/lib.rs @@ -67,6 +67,7 @@ bpf_file!( UPROBE_COOKIE => "uprobe_cookie", PRINTK_TEST => "printk_test", PROG_ARRAY => "prog_array", + SOCKET_FILTER => "socket_filter", ); #[cfg(test)] diff --git a/test/integration-test/src/tests.rs b/test/integration-test/src/tests.rs index 3750c6994..fe338ea08 100644 --- a/test/integration-test/src/tests.rs +++ b/test/integration-test/src/tests.rs @@ -35,6 +35,7 @@ mod relocations; mod ring_buf; mod sk_storage; mod smoke; +mod socket_filter; mod strncmp; mod tcx; mod uprobe_cookie; 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 000000000..fdc0e80b2 --- /dev/null +++ b/test/integration-test/src/tests/socket_filter.rs @@ -0,0 +1,8 @@ +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(); + prog.load().unwrap(); +} From 561633a615226e66d78657f9c19c9475ac1749ca 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 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ebpf/aya-ebpf/src/programs/sk_buff.rs b/ebpf/aya-ebpf/src/programs/sk_buff.rs index 5f10b2182..d77a859bb 100644 --- a/ebpf/aya-ebpf/src/programs/sk_buff.rs +++ b/ebpf/aya-ebpf/src/programs/sk_buff.rs @@ -7,7 +7,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, @@ -85,8 +85,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(