From 2432677b2bafcfa4028f9315f6754b14c070b4dd Mon Sep 17 00:00:00 2001 From: Michal Rostecki Date: Fri, 30 Sep 2022 07:07:31 +0200 Subject: [PATCH 1/3] Avoid integer to pointer casts Instead, operate on byte slices if possible. That's the first step in getting rid of miri warnings about Strict Provenance[0]. [0] https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance Signed-off-by: Michal Rostecki --- aya/src/obj/btf/types.rs | 12 ++++++------ aya/src/obj/mod.rs | 11 +++-------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/aya/src/obj/btf/types.rs b/aya/src/obj/btf/types.rs index d31dcca2..6d65330a 100644 --- a/aya/src/obj/btf/types.rs +++ b/aya/src/obj/btf/types.rs @@ -894,12 +894,12 @@ unsafe fn read_array(data: &[u8], len: usize) -> Result, BtfError> { if mem::size_of::() * len > data.len() { return Err(BtfError::InvalidTypeInfo); } - - Ok((0..len) - .map(|i| { - ptr::read_unaligned::((data.as_ptr() as usize + i * mem::size_of::()) as *const T) - }) - .collect::>()) + let data = &data[0..mem::size_of::() * len]; + let r = data + .chunks(mem::size_of::()) + .map(|chunk| ptr::read_unaligned(chunk.as_ptr() as *const T)) + .collect(); + Ok(r) } impl BtfType { diff --git a/aya/src/obj/mod.rs b/aya/src/obj/mod.rs index dca36b70..c5abe056 100644 --- a/aya/src/obj/mod.rs +++ b/aya/src/obj/mod.rs @@ -1348,15 +1348,10 @@ pub(crate) fn copy_instructions(data: &[u8]) -> Result, ParseError if data.len() % mem::size_of::() > 0 { return Err(ParseError::InvalidProgramCode); } - let num_instructions = data.len() / mem::size_of::(); - let instructions = (0..num_instructions) - .map(|i| unsafe { - ptr::read_unaligned( - (data.as_ptr() as usize + i * mem::size_of::()) as *const bpf_insn, - ) - }) + let instructions = data + .chunks_exact(mem::size_of::()) + .map(|d| unsafe { ptr::read_unaligned(d.as_ptr() as *const bpf_insn) }) .collect::>(); - Ok(instructions) } From 43aff5779390881d785a4d1c0d6c7bd681381dfe Mon Sep 17 00:00:00 2001 From: Michal Rostecki Date: Fri, 30 Sep 2022 17:50:57 +0200 Subject: [PATCH 2/3] maps: Disable miri warnings about integer-to-pointer conversions `override_syscall` performs integer-to-pointer conversion. This is considered harmful on the newest Rust nightly which provides `ptr::from_exposed_addr`, but there is no other way on Rust stable than doing `as *const T`, which is what miri is unhappy about. Signed-off-by: Michal Rostecki --- aya/src/maps/hash_map/hash_map.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs index 33bd6cc8..b0edd734 100644 --- a/aya/src/maps/hash_map/hash_map.rs +++ b/aya/src/maps/hash_map/hash_map.rs @@ -475,6 +475,10 @@ mod tests { } #[test] + // Syscall overrides are performing integer-to-pointer conversions, which + // should be done with `ptr::from_exposed_addr` in Rust nightly, but we have + // to support stable as well. + #[cfg_attr(miri, ignore)] fn test_keys() { override_syscall(|call| match call { Syscall::Bpf { @@ -497,6 +501,10 @@ mod tests { } #[test] + // Syscall overrides are performing integer-to-pointer conversions, which + // should be done with `ptr::from_exposed_addr` in Rust nightly, but we have + // to support stable as well. + #[cfg_attr(miri, ignore)] fn test_keys_error() { override_syscall(|call| match call { Syscall::Bpf { @@ -532,6 +540,10 @@ mod tests { } #[test] + // Syscall overrides are performing integer-to-pointer conversions, which + // should be done with `ptr::from_exposed_addr` in Rust nightly, but we have + // to support stable as well. + #[cfg_attr(miri, ignore)] fn test_iter() { override_syscall(|call| match call { Syscall::Bpf { @@ -556,6 +568,10 @@ mod tests { } #[test] + // Syscall overrides are performing integer-to-pointer conversions, which + // should be done with `ptr::from_exposed_addr` in Rust nightly, but we have + // to support stable as well. + #[cfg_attr(miri, ignore)] fn test_iter_key_deleted() { override_syscall(|call| match call { Syscall::Bpf { @@ -591,6 +607,10 @@ mod tests { } #[test] + // Syscall overrides are performing integer-to-pointer conversions, which + // should be done with `ptr::from_exposed_addr` in Rust nightly, but we have + // to support stable as well. + #[cfg_attr(miri, ignore)] fn test_iter_key_error() { override_syscall(|call| match call { Syscall::Bpf { @@ -632,6 +652,10 @@ mod tests { } #[test] + // Syscall overrides are performing integer-to-pointer conversions, which + // should be done with `ptr::from_exposed_addr` in Rust nightly, but we have + // to support stable as well. + #[cfg_attr(miri, ignore)] fn test_iter_value_error() { override_syscall(|call| match call { Syscall::Bpf { From 325391892c33fd92148951b5008cb53e7c1eaa31 Mon Sep 17 00:00:00 2001 From: Michal Rostecki Date: Mon, 3 Oct 2022 15:48:55 +0200 Subject: [PATCH 3/3] integration-test: Remove multimap C test libbpf 1.0 doesn't support multimaps defined in `maps` section, it supports only BTF maps. At the same time, we already test multimaps loading with the Rust example (`integration-ebpf/src/bpf/map_test.rs`), so we can just remove the failing C test. Signed-off-by: Michal Rostecki --- test/integration-ebpf/src/bpf/multimap.bpf.c | 47 -------------------- test/integration-test/src/tests/load.rs | 11 ----- 2 files changed, 58 deletions(-) delete mode 100644 test/integration-ebpf/src/bpf/multimap.bpf.c diff --git a/test/integration-ebpf/src/bpf/multimap.bpf.c b/test/integration-ebpf/src/bpf/multimap.bpf.c deleted file mode 100644 index 615935eb..00000000 --- a/test/integration-ebpf/src/bpf/multimap.bpf.c +++ /dev/null @@ -1,47 +0,0 @@ -#include -#include - -const int XDP_ACTION_MAX = (XDP_TX + 1); - -struct datarec { - __u64 rx_packets; -}; - -// stats keyed by XDP Action -struct bpf_map_def SEC("maps") xdp_stats_map = { - .type = BPF_MAP_TYPE_ARRAY, - .key_size = sizeof(__u32), - .value_size = sizeof(struct datarec), - .max_entries = XDP_ACTION_MAX, -}; - -// tracks number of times called -struct bpf_map_def SEC("maps") prog_stats_map = { - .type = BPF_MAP_TYPE_ARRAY, - .key_size = sizeof(__u32), - .value_size = sizeof(__u64), - .max_entries = 1, -}; - -SEC("xdp/stats") -int xdp_stats(struct xdp_md *ctx) -{ - __u64 *stats; - struct datarec *rec; - __u32 key = XDP_PASS; - __u32 k1 = 0; - - stats = bpf_map_lookup_elem(&prog_stats_map, &k1); - if (!stats) - return XDP_ABORTED; - __sync_fetch_and_add(stats, 1); - - rec = bpf_map_lookup_elem(&xdp_stats_map, &key); - if (!rec) - return XDP_ABORTED; - __sync_fetch_and_add(&rec->rx_packets, 1); - - return XDP_PASS; -} - -char _license[] SEC("license") = "GPL"; diff --git a/test/integration-test/src/tests/load.rs b/test/integration-test/src/tests/load.rs index 37ff0e8f..846ee8a9 100644 --- a/test/integration-test/src/tests/load.rs +++ b/test/integration-test/src/tests/load.rs @@ -27,17 +27,6 @@ fn long_name() -> anyhow::Result<()> { Ok(()) } -#[integration_test] -fn multiple_maps() -> anyhow::Result<()> { - let bytes = - include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/multimap.bpf.o"); - let mut bpf = Bpf::load(bytes)?; - let pass: &mut Xdp = bpf.program_mut("stats").unwrap().try_into().unwrap(); - pass.load().unwrap(); - pass.attach("lo", XdpFlags::default()).unwrap(); - Ok(()) -} - #[integration_test] fn multiple_btf_maps() -> anyhow::Result<()> { let bytes =