redbpf icon indicating copy to clipboard operation
redbpf copied to clipboard

Runtime random error due to bad assert in crate zero?

Open dlescos opened this issue 3 years ago • 2 comments

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 avatar Dec 31 '22 23:12 dlescos

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

  • https://github.com/nrc/zero/commit/3b6043ca976dc4dee34012f2cb11ce69f946ea20
  • https://github.com/nrc/zero/commit/1d571c9e4e844df5703cad164cb097e75f2828c1

dlescos avatar Jan 01 '23 13:01 dlescos

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

bendahl avatar May 03 '23 18:05 bendahl