From f41be0b03d54c2acba457f56b7e2b47185cecf37 Mon Sep 17 00:00:00 2001 From: Michal Rostecki Date: Tue, 14 Mar 2023 14:56:47 +0100 Subject: [PATCH] ebpf: Call `bpf_probe_read` on `*const T` BTF arguments It's necessary to call `bpf_probe_read` not only for pointers retrieved from `PtRegs`, but also from BTF arguments. `bpf_probe_read` might return an error, so the return type of `.arg()` methods in contexts handling BTF arguments changes from `T` to `Option`. `None` is returned when `bpf_probe_read` call is not successful. Fixes: #542 --- bpf/aya-bpf/src/args.rs | 13 ++++++---- bpf/aya-bpf/src/programs/fentry.rs | 2 +- bpf/aya-bpf/src/programs/fexit.rs | 2 +- bpf/aya-bpf/src/programs/lsm.rs | 2 +- bpf/aya-bpf/src/programs/tp_btf.rs | 2 +- test/integration-ebpf/Cargo.toml | 4 +++ test/integration-ebpf/src/args.rs | 33 ++++++++++++++++++++++++ test/integration-test/src/tests/args.rs | 34 +++++++++++++++++++++++++ test/integration-test/src/tests/mod.rs | 1 + 9 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 test/integration-ebpf/src/args.rs create mode 100644 test/integration-test/src/tests/args.rs diff --git a/bpf/aya-bpf/src/args.rs b/bpf/aya-bpf/src/args.rs index fea024eb..184aaae5 100644 --- a/bpf/aya-bpf/src/args.rs +++ b/bpf/aya-bpf/src/args.rs @@ -21,14 +21,17 @@ pub unsafe trait FromBtfArgument: Sized { /// memory. In particular, the value of `n` must not exceed the number of function /// arguments. Moreover, `ctx` must be a valid pointer to a BTF context, and `T` must /// be the right type for the given argument. - unsafe fn from_argument(ctx: *const c_void, n: usize) -> Self; + unsafe fn from_argument(ctx: *const c_void, n: usize) -> Option; } unsafe impl FromBtfArgument for *const T { - unsafe fn from_argument(ctx: *const c_void, n: usize) -> *const T { + unsafe fn from_argument(ctx: *const c_void, n: usize) -> Option { // BTF arguments are exposed as an array of `usize` where `usize` can // either be treated as a pointer or a primitive type - *(ctx as *const usize).add(n) as _ + // *(ctx as *const usize).add(n) as _ + bpf_probe_read((ctx as *const usize).add(n)) + .map(|v| v as *const _) + .ok() } } @@ -36,10 +39,10 @@ unsafe impl FromBtfArgument for *const T { macro_rules! unsafe_impl_from_btf_argument { ($type:ident) => { unsafe impl FromBtfArgument for $type { - unsafe fn from_argument(ctx: *const c_void, n: usize) -> Self { + unsafe fn from_argument(ctx: *const c_void, n: usize) -> Option { // BTF arguments are exposed as an array of `usize` where `usize` can // either be treated as a pointer or a primitive type - *(ctx as *const usize).add(n) as _ + Some(*(ctx as *const usize).add(n) as _) } } }; diff --git a/bpf/aya-bpf/src/programs/fentry.rs b/bpf/aya-bpf/src/programs/fentry.rs index 56687539..ddd27b5f 100644 --- a/bpf/aya-bpf/src/programs/fentry.rs +++ b/bpf/aya-bpf/src/programs/fentry.rs @@ -31,7 +31,7 @@ impl FEntryContext { /// Ok(0) /// } /// ``` - pub unsafe fn arg(&self, n: usize) -> T { + pub unsafe fn arg(&self, n: usize) -> Option { T::from_argument(self.ctx as *const _, n) } } diff --git a/bpf/aya-bpf/src/programs/fexit.rs b/bpf/aya-bpf/src/programs/fexit.rs index 1e52d733..e5691001 100644 --- a/bpf/aya-bpf/src/programs/fexit.rs +++ b/bpf/aya-bpf/src/programs/fexit.rs @@ -31,7 +31,7 @@ impl FExitContext { /// Ok(0) /// } /// ``` - pub unsafe fn arg(&self, n: usize) -> T { + pub unsafe fn arg(&self, n: usize) -> Option { T::from_argument(self.ctx as *const _, n) } } diff --git a/bpf/aya-bpf/src/programs/lsm.rs b/bpf/aya-bpf/src/programs/lsm.rs index 4cb76cf4..8c7c1641 100644 --- a/bpf/aya-bpf/src/programs/lsm.rs +++ b/bpf/aya-bpf/src/programs/lsm.rs @@ -50,7 +50,7 @@ impl LsmContext { /// ``` /// /// [1]: https://elixir.bootlin.com/linux/latest/source/include/linux/lsm_hook_defs.h - pub unsafe fn arg(&self, n: usize) -> T { + pub unsafe fn arg(&self, n: usize) -> Option { T::from_argument(self.ctx as *const _, n) } } diff --git a/bpf/aya-bpf/src/programs/tp_btf.rs b/bpf/aya-bpf/src/programs/tp_btf.rs index f8a1b989..2e9dee00 100644 --- a/bpf/aya-bpf/src/programs/tp_btf.rs +++ b/bpf/aya-bpf/src/programs/tp_btf.rs @@ -40,7 +40,7 @@ impl BtfTracePointContext { /// ``` /// /// [1]: https://elixir.bootlin.com/linux/latest/source/include/linux/lsm_hook_defs.h - pub unsafe fn arg(&self, n: usize) -> T { + pub unsafe fn arg(&self, n: usize) -> Option { T::from_argument(self.ctx as *const _, n) } } diff --git a/test/integration-ebpf/Cargo.toml b/test/integration-ebpf/Cargo.toml index 8668b91d..82145d18 100644 --- a/test/integration-ebpf/Cargo.toml +++ b/test/integration-ebpf/Cargo.toml @@ -7,6 +7,10 @@ publish = false [dependencies] aya-bpf = { path = "../../bpf/aya-bpf" } +[[bin]] +name = "args" +path = "src/args.rs" + [[bin]] name = "map_test" path = "src/map_test.rs" diff --git a/test/integration-ebpf/src/args.rs b/test/integration-ebpf/src/args.rs new file mode 100644 index 00000000..f759f7f4 --- /dev/null +++ b/test/integration-ebpf/src/args.rs @@ -0,0 +1,33 @@ +#![no_std] +#![no_main] + +use aya_bpf::{ + cty::{c_long, c_longlong}, + macros::{fentry, kprobe}, + programs::{FEntryContext, ProbeContext}, +}; + +#[kprobe] +pub fn kprobe_vfs_write(ctx: ProbeContext) { + let _ = try_kprobe_vfs_write(ctx); +} + +fn try_kprobe_vfs_write(ctx: ProbeContext) -> Result<(), c_long> { + let _pos: *const c_longlong = ctx.arg(3).ok_or(1)?; + Ok(()) +} + +#[fentry] +pub fn fentry_vfs_write(ctx: FEntryContext) { + let _ = try_fentry_vfs_write(ctx); +} + +fn try_fentry_vfs_write(ctx: FEntryContext) -> Result<(), c_long> { + let _pos: *const c_longlong = unsafe { ctx.arg(3).ok_or(1)? }; + Ok(()) +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + unsafe { core::hint::unreachable_unchecked() } +} diff --git a/test/integration-test/src/tests/args.rs b/test/integration-test/src/tests/args.rs new file mode 100644 index 00000000..947ee4ba --- /dev/null +++ b/test/integration-test/src/tests/args.rs @@ -0,0 +1,34 @@ +use aya::{ + include_bytes_aligned, + programs::{FEntry, KProbe}, + Bpf, Btf, +}; + +use super::{integration_test, IntegrationTest}; + +#[integration_test] +fn kprobe_args() { + let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/args"); + let mut bpf = Bpf::load(bytes).unwrap(); + let kprobe_vfs_write: &mut KProbe = bpf + .program_mut("kprobe_vfs_write") + .unwrap() + .try_into() + .unwrap(); + kprobe_vfs_write.load().unwrap(); + kprobe_vfs_write.attach("vfs_write", 0).unwrap(); +} + +#[integration_test] +fn fentry_args() { + let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/args"); + let mut bpf = Bpf::load(bytes).unwrap(); + let fentry_vfs_write: &mut FEntry = bpf + .program_mut("fentry_vfs_write") + .unwrap() + .try_into() + .unwrap(); + let btf = Btf::from_sys_fs().unwrap(); + fentry_vfs_write.load("vfs_write", &btf).unwrap(); + fentry_vfs_write.attach().unwrap(); +} diff --git a/test/integration-test/src/tests/mod.rs b/test/integration-test/src/tests/mod.rs index ddb1b504..e26baf51 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 args; pub mod elf; pub mod load; pub mod rbpf;