From d6b976c6f1f6163680c179502f4f454d0cec747e Mon Sep 17 00:00:00 2001 From: Alessandro Decina Date: Wed, 30 Nov 2022 19:10:57 +1100 Subject: [PATCH 1/2] aya: btf: switch ComputedRelocationValue::value to u64 This is in preparation of adding Enum64 relocation support --- aya-obj/src/btf/relocation.rs | 34 +++++++++---------- .../integration-test/src/tests/relocations.rs | 25 +++++--------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/aya-obj/src/btf/relocation.rs b/aya-obj/src/btf/relocation.rs index f70502b0..05551dd0 100644 --- a/aya-obj/src/btf/relocation.rs +++ b/aya-obj/src/btf/relocation.rs @@ -786,7 +786,7 @@ struct ComputedRelocation { #[derive(Debug)] struct ComputedRelocationValue { - value: u32, + value: u64, size: u32, type_id: Option, } @@ -854,7 +854,7 @@ impl ComputedRelocation { ins.imm = target_value as i32; } BPF_LDX | BPF_ST | BPF_STX => { - if target_value > i16::MAX as u32 { + if target_value > i16::MAX as u64 { return Err(RelocationError::InvalidInstruction { relocation_number: rel.number, index: ins_index, @@ -914,7 +914,7 @@ impl ComputedRelocation { }, )?; - next_ins.imm = 0; + next_ins.imm = (target_value >> 32) as i32; } class => { return Err(RelocationError::InvalidInstruction { @@ -934,11 +934,11 @@ impl ComputedRelocation { ) -> Result { use RelocationKind::*; let value = match (rel.kind, spec) { - (EnumVariantExists, spec) => spec.is_some() as u32, + (EnumVariantExists, spec) => spec.is_some() as u64, (EnumVariantValue, Some(spec)) => { let accessor = &spec.accessors[0]; match spec.btf.type_by_id(accessor.type_id)? { - BtfType::Enum(en) => en.variants[accessor.index].value as u32, + BtfType::Enum(en) => en.variants[accessor.index].value as u64, // candidate selection ensures that rel_kind == local_kind == target_kind _ => unreachable!(), } @@ -969,7 +969,7 @@ impl ComputedRelocation { // this is the bpf_preserve_field_info(member_access, FIELD_EXISTENCE) case. If we // managed to build a spec, it means the field exists. return Ok(ComputedRelocationValue { - value: spec.is_some() as u32, + value: spec.is_some() as u64, size: 0, type_id: None, }); @@ -991,12 +991,12 @@ impl ComputedRelocation { // the last accessor is unnamed, meaning that this is an array access return match rel.kind { FieldByteOffset => Ok(ComputedRelocationValue { - value: (spec.bit_offset / 8) as u32, + value: (spec.bit_offset / 8) as u64, size: spec.btf.type_size(accessor.type_id)? as u32, type_id: Some(accessor.type_id), }), FieldByteSize => Ok(ComputedRelocationValue { - value: spec.btf.type_size(accessor.type_id)? as u32, + value: spec.btf.type_size(accessor.type_id)? as u64, size: 0, type_id: Some(accessor.type_id), }), @@ -1061,30 +1061,30 @@ impl ComputedRelocation { #[allow(clippy::wildcard_in_or_patterns)] match rel.kind { FieldByteOffset => { - value.value = byte_off; + value.value = byte_off as u64; if !is_bitfield { value.size = byte_size; value.type_id = Some(member_type_id); } } FieldByteSize => { - value.value = byte_size; + value.value = byte_size as u64; } FieldSigned => match member_ty { BtfType::Enum(_) => value.value = 1, - BtfType::Int(i) => value.value = i.encoding() as u32 & IntEncoding::Signed as u32, + BtfType::Int(i) => value.value = i.encoding() as u64 & IntEncoding::Signed as u64, _ => (), }, #[cfg(target_endian = "little")] FieldLShift64 => { - value.value = 64 - (bit_off + bit_size - byte_off * 8); + value.value = 64 - (bit_off + bit_size - byte_off * 8) as u64; } #[cfg(target_endian = "big")] FieldLShift64 => { value.value = (8 - byte_size) * 8 + (bit_off - byte_off * 8); } FieldRShift64 => { - value.value = 64 - bit_size; + value.value = 64 - bit_size as u64; } FieldExists // this is handled at the start of the function | _ => panic!("bug! this should not be reached"), @@ -1101,11 +1101,11 @@ impl ComputedRelocation { use RelocationKind::*; let value = match (rel.kind, target_spec) { - (TypeIdLocal, _) => local_spec.root_type_id, - (TypeIdTarget, Some(target_spec)) => target_spec.root_type_id, - (TypeExists, target_spec) => target_spec.is_some() as u32, + (TypeIdLocal, _) => local_spec.root_type_id as u64, + (TypeIdTarget, Some(target_spec)) => target_spec.root_type_id as u64, + (TypeExists, target_spec) => target_spec.is_some() as u64, (TypeSize, Some(target_spec)) => { - target_spec.btf.type_size(target_spec.root_type_id)? as u32 + target_spec.btf.type_size(target_spec.root_type_id)? as u64 } _ => { return Err(RelocationError::MissingTargetDefinition { diff --git a/test/integration-test/src/tests/relocations.rs b/test/integration-test/src/tests/relocations.rs index bc2c1883..198001bd 100644 --- a/test/integration-test/src/tests/relocations.rs +++ b/test/integration-test/src/tests/relocations.rs @@ -28,9 +28,7 @@ fn relocate_field() { relocation_code: r#" __u8 memory[] = {1, 2, 3, 4}; struct foo *ptr = (struct foo *) &memory; - bpf_probe_read_kernel(&value, - sizeof(__u8), - __builtin_preserve_access_index(&ptr->c)); + value = __builtin_preserve_access_index(ptr->c); "#, } .build() @@ -73,9 +71,7 @@ fn relocate_pointer() { relocation_code: r#" __u8 memory[] = {42, 0, 0, 0, 0, 0, 0, 0}; struct bar* ptr = (struct bar *) &memory; - bpf_probe_read_kernel(&value, - sizeof(void *), - __builtin_preserve_access_index(&ptr->f)); + value = (__u64) __builtin_preserve_access_index(ptr->f); "#, } .build() @@ -121,14 +117,13 @@ impl RelocationTest { #include static long (*bpf_map_update_elem)(void *map, const void *key, const void *value, __u64 flags) = (void *) 2; - static long (*bpf_probe_read_kernel)(void *dst, __u32 size, const void *unsafe_ptr) = (void *) 113; {local_definition} struct {{ int (*type)[BPF_MAP_TYPE_ARRAY]; __u32 *key; - __u32 *value; + __u64 *value; int (*max_entries)[1]; }} output_map __attribute__((section(".maps"), used)); @@ -136,7 +131,7 @@ impl RelocationTest { __attribute__((section("tracepoint/bpf_prog"), used)) int bpf_prog(void *ctx) {{ __u32 key = 0; - __u32 value = 0; + __u64 value = 0; {relocation_code} bpf_map_update_elem(&output_map, &key, &value, BPF_ANY); return 0; @@ -165,11 +160,9 @@ impl RelocationTest { r#" #include - static long (*bpf_probe_read_kernel)(void *dst, __u32 size, const void *unsafe_ptr) = (void *) 113; - {target_btf} int main() {{ - __u32 value = 0; + __u64 value = 0; // This is needed to make sure to emit BTF for the defined types, // it could be dead code eliminated if we don't. {relocation_code}; @@ -218,17 +211,17 @@ struct RelocationTestRunner { impl RelocationTestRunner { /// Run test and return the output value - fn run(&self) -> Result { + fn run(&self) -> Result { self.run_internal(true).context("Error running with BTF") } /// Run without loading btf - fn run_no_btf(&self) -> Result { + fn run_no_btf(&self) -> Result { self.run_internal(false) .context("Error running without BTF") } - fn run_internal(&self, with_relocations: bool) -> Result { + fn run_internal(&self, with_relocations: bool) -> Result { let mut loader = BpfLoader::new(); if with_relocations { loader.btf(Some(&self.btf)); @@ -250,7 +243,7 @@ impl RelocationTestRunner { // To inspect the loaded eBPF bytecode, increse the timeout and run: // $ sudo bpftool prog dump xlated name bpf_prog - let output_map: Array<_, u32> = bpf.take_map("output_map").unwrap().try_into().unwrap(); + let output_map: Array<_, u64> = bpf.take_map("output_map").unwrap().try_into().unwrap(); let key = 0; output_map.get(&key, 0).context("Getting key 0 failed") } From 4482db42d86c657826efe80f484f57a601ed2f38 Mon Sep 17 00:00:00 2001 From: Alessandro Decina Date: Thu, 26 Jan 2023 15:36:01 +1100 Subject: [PATCH 2/2] aya: btf: fix relocations for signed enums (32 bits) Enums now carry a signed bit in the info flags. Take it into account when applying enum relocations. --- aya-obj/src/btf/btf.rs | 6 ++-- aya-obj/src/btf/relocation.rs | 11 +++++-- aya-obj/src/btf/types.rs | 8 +++-- .../integration-test/src/tests/relocations.rs | 31 ++++++++++++++++--- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/aya-obj/src/btf/btf.rs b/aya-obj/src/btf/btf.rs index 61fa6a52..f439ee1a 100644 --- a/aya-obj/src/btf/btf.rs +++ b/aya-obj/src/btf/btf.rs @@ -529,7 +529,7 @@ impl Btf { .iter() .map(|p| BtfEnum { name_offset: p.name_offset, - value: p.btf_type as i32, + value: p.btf_type, }) .collect(); let enum_type = BtfType::Enum(Enum::new(ty.name_offset, members)); @@ -1246,9 +1246,9 @@ mod tests { assert!(fixed.name_offset == 0); assert!(fixed.variants.len() == 2); assert!(btf.string_at(fixed.variants[0].name_offset).unwrap() == "a"); - assert!(fixed.variants[0].value == int_type_id as i32); + assert!(fixed.variants[0].value == int_type_id); assert!(btf.string_at(fixed.variants[1].name_offset).unwrap() == "b"); - assert!(fixed.variants[1].value == int_type_id as i32); + assert!(fixed.variants[1].value == int_type_id); } else { panic!("not an emum") } diff --git a/aya-obj/src/btf/relocation.rs b/aya-obj/src/btf/relocation.rs index 05551dd0..4a618c0a 100644 --- a/aya-obj/src/btf/relocation.rs +++ b/aya-obj/src/btf/relocation.rs @@ -938,7 +938,14 @@ impl ComputedRelocation { (EnumVariantValue, Some(spec)) => { let accessor = &spec.accessors[0]; match spec.btf.type_by_id(accessor.type_id)? { - BtfType::Enum(en) => en.variants[accessor.index].value as u64, + BtfType::Enum(en) => { + let value = en.variants[accessor.index].value; + if en.is_signed() { + value as i32 as u64 + } else { + value as u64 + } + } // candidate selection ensures that rel_kind == local_kind == target_kind _ => unreachable!(), } @@ -1071,7 +1078,7 @@ impl ComputedRelocation { value.value = byte_size as u64; } FieldSigned => match member_ty { - BtfType::Enum(_) => value.value = 1, + BtfType::Enum(en) => value.value = en.is_signed() as u64, BtfType::Int(i) => value.value = i.encoding() as u64 & IntEncoding::Signed as u64, _ => (), }, diff --git a/aya-obj/src/btf/types.rs b/aya-obj/src/btf/types.rs index 72474f7e..629e54f5 100644 --- a/aya-obj/src/btf/types.rs +++ b/aya-obj/src/btf/types.rs @@ -389,7 +389,7 @@ impl Int { #[derive(Debug, Clone)] pub(crate) struct BtfEnum { pub(crate) name_offset: u32, - pub(crate) value: i32, + pub(crate) value: u32, } #[repr(C)] @@ -409,7 +409,7 @@ impl Enum { buf.extend(bytes_of::(&self.size)); for v in &self.variants { buf.extend(bytes_of::(&v.name_offset)); - buf.extend(bytes_of::(&v.value)); + buf.extend(bytes_of::(&v.value)); } buf } @@ -432,6 +432,10 @@ impl Enum { variants, } } + + pub(crate) fn is_signed(&self) -> bool { + self.info >> 31 == 1 + } } #[repr(C)] diff --git a/test/integration-test/src/tests/relocations.rs b/test/integration-test/src/tests/relocations.rs index 198001bd..74eee2f9 100644 --- a/test/integration-test/src/tests/relocations.rs +++ b/test/integration-test/src/tests/relocations.rs @@ -6,6 +6,9 @@ use aya::{maps::Array, programs::TracePoint, BpfLoader, Btf, Endianness}; use super::{integration_test, IntegrationTest}; +// In the tests below we often use values like 0xAAAAAAAA or -0x7AAAAAAA. Those values have no +// special meaning, they just have "nice" bit patterns that can be helpful while debugging. + #[integration_test] fn relocate_field() { let test = RelocationTest { @@ -41,10 +44,30 @@ fn relocate_field() { fn relocate_enum() { let test = RelocationTest { local_definition: r#" - enum foo { D = 1 }; + enum foo { D = 0xAAAAAAAA }; + "#, + target_btf: r#" + enum foo { D = 0xBBBBBBBB } e1; + "#, + relocation_code: r#" + #define BPF_ENUMVAL_VALUE 1 + value = __builtin_preserve_enum_value(*(typeof(enum foo) *)D, BPF_ENUMVAL_VALUE); + "#, + } + .build() + .unwrap(); + assert_eq!(test.run().unwrap(), 0xBBBBBBBB); + assert_eq!(test.run_no_btf().unwrap(), 0xAAAAAAAA); +} + +#[integration_test] +fn relocate_enum_signed() { + let test = RelocationTest { + local_definition: r#" + enum foo { D = -0x7AAAAAAA }; "#, target_btf: r#" - enum foo { D = 4 } e1; + enum foo { D = -0x7BBBBBBB } e1; "#, relocation_code: r#" #define BPF_ENUMVAL_VALUE 1 @@ -53,8 +76,8 @@ fn relocate_enum() { } .build() .unwrap(); - assert_eq!(test.run().unwrap(), 4); - assert_eq!(test.run_no_btf().unwrap(), 1); + assert_eq!(test.run().unwrap() as i64, -0x7BBBBBBBi64); + assert_eq!(test.run_no_btf().unwrap() as i64, -0x7AAAAAAAi64); } #[integration_test]