From adf16e2102d94e46630572c6d18d10893697106c Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Fri, 1 Nov 2024 11:27:55 +0000 Subject: [PATCH 1/3] test: Reproduce relocation bug Users have reported issues with programs failing the verifier when they are attempting to read or write to variables that the compiler places in the .bss section. Add a test that places variables in each section and exercises read and write operations on them. Signed-off-by: Dave Tucker --- .../bpf/variables_reloc.bpf.c | 21 +++++++++++++++++++ test/integration-test/build.rs | 1 + test/integration-test/src/lib.rs | 2 ++ .../integration-test/src/tests/relocations.rs | 17 ++++++++++++++- 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/integration-test/bpf/variables_reloc.bpf.c diff --git a/test/integration-test/bpf/variables_reloc.bpf.c b/test/integration-test/bpf/variables_reloc.bpf.c new file mode 100644 index 00000000..08712937 --- /dev/null +++ b/test/integration-test/bpf/variables_reloc.bpf.c @@ -0,0 +1,21 @@ +// clang-format off +#include +#include +// clang-format on + +volatile unsigned int key1 = 0; // .bss +volatile unsigned int key2 = 1; // .data +volatile const unsigned int key3 = 2; // .rodata + +SEC("xdp") +int variables_reloc(struct xdp_md *ctx) { + if (key1 == 0 && key2 != 1 && key3 != 2) { + key1 += 1; + key2 += 1; + return XDP_DROP; + } else { + return XDP_PASS; + } +} + +char _license[] SEC("license") = "GPL"; diff --git a/test/integration-test/build.rs b/test/integration-test/build.rs index 8e53b5a9..5266a8ea 100644 --- a/test/integration-test/build.rs +++ b/test/integration-test/build.rs @@ -71,6 +71,7 @@ fn main() { ("multimap-btf.bpf.c", false), ("reloc.bpf.c", true), ("text_64_64_reloc.c", false), + ("variables_reloc.bpf.c", false), ]; if build_integration_bpf { diff --git a/test/integration-test/src/lib.rs b/test/integration-test/src/lib.rs index c20926dd..ffed5dd4 100644 --- a/test/integration-test/src/lib.rs +++ b/test/integration-test/src/lib.rs @@ -9,6 +9,8 @@ pub const RELOC_BTF: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/reloc.bpf.target.o")); pub const TEXT_64_64_RELOC: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/text_64_64_reloc.o")); +pub const VARIABLES_RELOC: &[u8] = + include_bytes_aligned!(concat!(env!("OUT_DIR"), "/variables_reloc.bpf.o")); pub const LOG: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/log")); pub const MAP_TEST: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/map_test")); diff --git a/test/integration-test/src/tests/relocations.rs b/test/integration-test/src/tests/relocations.rs index eb43466a..2dce6a60 100644 --- a/test/integration-test/src/tests/relocations.rs +++ b/test/integration-test/src/tests/relocations.rs @@ -1,4 +1,8 @@ -use aya::{programs::UProbe, util::KernelVersion, Ebpf}; +use aya::{ + programs::{UProbe, Xdp}, + util::KernelVersion, + Ebpf, +}; use test_log::test; #[test] @@ -33,6 +37,17 @@ fn text_64_64_reloc() { assert_eq!(m.get(&1, 0).unwrap(), 3); } +#[test] +fn variables_reloc() { + let mut bpf = Ebpf::load(crate::VARIABLES_RELOC).unwrap(); + let prog: &mut Xdp = bpf + .program_mut("variables_reloc") + .unwrap() + .try_into() + .unwrap(); + prog.load().unwrap(); +} + fn load_and_attach(name: &str, bytes: &[u8]) -> Ebpf { let mut bpf = Ebpf::load(bytes).unwrap(); From 3aa274597236e8c9938e3626346e2d23119e7ed6 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Fri, 1 Nov 2024 11:29:24 +0000 Subject: [PATCH 2/3] test(init): run test with debug logs This provides more useful failure messages when integration tests fail Signed-off-by: Dave Tucker --- init/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/init/src/main.rs b/init/src/main.rs index ddb46e64..524fa972 100644 --- a/init/src/main.rs +++ b/init/src/main.rs @@ -128,6 +128,7 @@ fn run() -> anyhow::Result<()> { let path = entry.path(); let status = std::process::Command::new(&path) .args(&args) + .env("RUST_LOG", "debug") .status() .with_context(|| format!("failed to execute {}", path.display()))?; From ca0c32d1076af81349a52235a4b6fb3937a697b3 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Fri, 1 Nov 2024 12:07:06 +0000 Subject: [PATCH 3/3] fix(aya): Fill bss maps with zeros The loader should fill bss maps with zeros according to the size of the ELF section. Failure to do so yields weird verifier messages as follows: ``` cannot access ptr member ops with moff 0 in struct bpf_map with off 0 size 4 ``` Reference to this in the cilium/ebpf code is here [1]. I could not find a reference in libbpf. 1: https://github.com/cilium/ebpf/blob/d0c8fc19376a9276cf5310c288d1eae99ed39eb3/elf_reader.go#L1159-L1165 Signed-off-by: Dave Tucker --- aya-obj/src/obj.rs | 14 +++++++++++++- aya/src/maps/mod.rs | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/aya-obj/src/obj.rs b/aya-obj/src/obj.rs index f0168950..bb1a3d92 100644 --- a/aya-obj/src/obj.rs +++ b/aya-obj/src/obj.rs @@ -5,6 +5,7 @@ use alloc::{ collections::BTreeMap, ffi::CString, string::{String, ToString}, + vec, vec::Vec, }; use core::{ffi::CStr, mem, ptr, slice::from_raw_parts_mut, str::FromStr}; @@ -1193,7 +1194,7 @@ fn get_map_field(btf: &Btf, type_id: u32) -> Result { // bytes and are relocated based on their section index. fn parse_data_map_section(section: &Section) -> Result { let (def, data) = match section.kind { - EbpfSectionKind::Bss | EbpfSectionKind::Data | EbpfSectionKind::Rodata => { + EbpfSectionKind::Data | EbpfSectionKind::Rodata => { let def = bpf_map_def { map_type: BPF_MAP_TYPE_ARRAY as u32, key_size: mem::size_of::() as u32, @@ -1210,6 +1211,17 @@ fn parse_data_map_section(section: &Section) -> Result { }; (def, section.data.to_vec()) } + EbpfSectionKind::Bss => { + let def = bpf_map_def { + map_type: BPF_MAP_TYPE_ARRAY as u32, + key_size: mem::size_of::() as u32, + value_size: section.size as u32, + max_entries: 1, + map_flags: 0, + ..Default::default() + }; + (def, vec![0; section.size as usize]) + } _ => unreachable!(), }; Ok(Map::Legacy(LegacyMap { diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 6f36b6b3..bb641288 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -637,7 +637,7 @@ impl MapData { pub(crate) fn finalize(&mut self) -> Result<(), MapError> { let Self { obj, fd } = self; - if !obj.data().is_empty() && obj.section_kind() != EbpfSectionKind::Bss { + if !obj.data().is_empty() { bpf_map_update_elem_ptr(fd.as_fd(), &0 as *const _, obj.data_mut().as_mut_ptr(), 0) .map_err(|(_, io_error)| SyscallError { call: "bpf_map_update_elem",