ebpf: Call bpf_probe_read on *const T BTF arguments

[vadorovsky]

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 Self to Option<Self>. None is returned when bpf_probe_read call is not successful.

Fixes: #542

---

This change is 

[netlify]

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	3141094
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/677d655ff1d17e000878d12c
😎 Deploy Preview	https://deploy-preview-543--aya-rs-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[tamird]

@vadorovsky mind rebasing?

[mergify]

@vadorovsky, this pull request is now in conflict and requires a rebase.

[mergify]

@vadorovsky, this pull request is now in conflict and requires a rebase.

[kamyuentse]

LGTM. I think this should be merged. @vadorovsky mind rebasing?

[tamird]

I rebased this, but I think the tests need more work.

[tamird]

Reviewed 7 of 9 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @vadorovsky)

---

test/integration-test/src/tests/args.rs line 1 at r2 (raw file):

use aya::{

not clear to me what we're testing here

---

test/integration-ebpf/src/args.rs line 11 at r2 (raw file):

#[kprobe]
pub fn kprobe_vfs_write(ctx: ProbeContext) {
    let _: Option<usize> = ctx.arg(3);

what exactly are we testing here?

[tamird]

Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @vadorovsky)

[vadorovsky]

After giving it some more thought, I'm closing it.

My initial intention behind the PR was handling the FEntry arguments in a similar way as we handle KProbe arguments. In case of KProbe, we call bpf_probe_read on each accessed register:

aya/ebpf/aya-ebpf/src/args.rs

Lines 107 to 118 in f34d355

	impl<T> FromPtRegs for *const T {
	fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
	match n {
	0 => unsafe { bpf_probe_read(&ctx.rdi).map(|v| v as *const _).ok() },
	1 => unsafe { bpf_probe_read(&ctx.rsi).map(|v| v as *const _).ok() },
	2 => unsafe { bpf_probe_read(&ctx.rdx).map(|v| v as *const _).ok() },
	3 => unsafe { bpf_probe_read(&ctx.rcx).map(|v| v as *const _).ok() },
	4 => unsafe { bpf_probe_read(&ctx.r8).map(|v| v as *const _).ok() },
	5 => unsafe { bpf_probe_read(&ctx.r9).map(|v| v as *const _).ok() },
	_ => None,
	}
	}

We don't do that for BTF arguments:

aya/ebpf/aya-ebpf/src/args.rs

Lines 41 to 52 in f34d355

	/// Helper macro to implement [`FromBtfArgument`] for a primitive type.
	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 {
	// 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 _
	}
	}
	};
	}

Therefore, when writing an FProbe, you often need to bpf_probe_read_kernel some pointers. For example, this program works:

#[fentry(function = "vfs_write")]
pub fn vfs_write(ctx: FEntryContext) -> u32 {
    match try_vfs_write(ctx) {
        Ok(_) => 0,
        Err(ret) => ret,
    }
}

fn try_vfs_write(ctx: FEntryContext) -> Result<(), u32> {
    let size: size_t = unsafe { ctx.arg(2) };
    let pos: *const i64 = unsafe { ctx.arg(3) };
    let pos = unsafe { bpf_probe_read_kernel(pos).map_err(|_| 1u32)? };
    info!(&ctx, "size={},pos={}", size, pos);
    Ok(())
}

But without bpf_probe_read_kernel:

    let pos: *const i64 = unsafe { ctx.arg(3) };
    let pos = unsafe { *pos };

It triggers the verifier error:

Error: the BPF_PROG_LOAD syscall failed. Verifier output: 0: R1=ctx() R10=fp0
0: (bf) r6 = r1                       ; R1=ctx() R6_w=ctx()
1: (79) r1 = *(u64 *)(r6 +16)         ; R1_w=scalar() R6_w=ctx()
2: (7b) *(u64 *)(r10 -16) = r1        ; R1_w=scalar(id=1) R10=fp0 fp-16_w=scalar(id=1)
3: (79) r1 = *(u64 *)(r6 +24)         ; R1_w=scalar() R6_w=ctx()
4: (79) r1 = *(u64 *)(r1 +0)
R1 invalid mem access 'scalar'
verification time 47 usec
stack depth 16+0+0+0

My initial though was - *let's make user's life easier and just do bpf_probe_read behind the scenes`.

But not I realized that:

• libbpf is not doing that for BTF argument based programs.
• The situation is not really analogic here (we are reading BTF arguments, not pt_regs pointer fields).
• Do we really care some much about requiring usage of bpf_probe_read? 🤔

So I'm closing the PR.