foniod / redbpf

Rust library for building and running BPF/eBPF modules
Apache License 2.0
1.71k stars 136 forks source link

Runtime random error due to bad assert in crate zero? #358

Open dlescos opened 1 year ago

dlescos commented 1 year ago

I got a strange error while testing the tutorial, related to the assert that checks for the alignment in the read_array function of the zero crate used by redbpf:

pub fn read_array<T: Pod>(input: &[u8]) -> &[T] {
    let t_size = mem::size_of::<T>();
    assert!(t_size > 0, "Can't read arrays of zero-sized types");
    assert!(input.len() % t_size == 0);
    let addr = input.as_ptr() as usize;
    assert!(addr & (mem::align_of::<T>() - 1) == 0); // We speak of this assert that checks the alignment

    unsafe { read_array_unsafe(input) }
}

The error happens randomly, for example while commenting out the code that attach the probe to do_sys_openat2.

I think redbpf was not affected until recently, as they pushed it a few days ago to crate.io (as explained here).

A quick fix would be to call read_array_unsafe instead of read_array (same with all read occurrences, if any).

But I'm not sure I understand why this assert fails.

dlescos commented 1 year ago

Maybe we should have a look at the comments of those commits:

bendahl commented 1 year ago

Hi @dlescos,

I was encountering the same problem you're describing. Thanks for the details you've posted! This helped a lot in finding an actual solution to the problem. While my first attempt at this was to simply use a patched version of the zero crate that got rid of the check, this clearly doesn't fix the underlying alignment issue.

Long story short, it seems like the folks over at aya were facing similar problems and found a solution in the form of a macro that ensures correct alignment of the included bpf program (byte array). The relevant code is located here:

https://github.com/aya-rs/aya/blob/bcb2972a969f85e8c6c77e1213d89cc8198e8fe7/aya/src/util.rs#L113-L148

@rsdy: So far I've only tested this locally using Rust 1.60, using llvm 13 to build the bpf code and llvm 14 for the rest. Testing has been done on Ubuntu 22.04 as well as Debian Buster. I cannot say for sure that this will work in all supported environments. However, if this works, it would be preferable to reverting the zero dependency to the previous version or patching it, effectively ignoring potential trouble at runtime.

Hope this helps.

Cheers, Ben