Repository: aya-rs/aya (language: rust)

Issue:
Relocations are too strict
I'm trying to make use of struct "flavors" but this cannot work with aya current design.

The following eBPF doesn't load with aya (full reproducer [here](https://github.com/marysaka/aya_struct_flavor_reloc_repro)):
```c
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
#include <bpf/bpf_core_read.h>

struct task_struct_rh___redhat {
    u64 self_exec_id;
};

struct task_struct___redhat {
    struct task_struct_rh___redhat *task_struct_rh;
};

#define printk(fmt, ...) ({                                    \
        char __fmt[] = fmt;                                    \
        bpf_trace_printk(__fmt, sizeof(__fmt), ##__VA_ARGS__); \
})

SEC("tp/bpf_prog")
int
bpf_prog(
    void *ctx
) {
    struct task_struct *task;
    u64 exec_id;

    task = (struct task_struct *)bpf_get_current_task();
    if (!task) {
        return 0;
    }

    // On CentOS 7 kernel, self_exec_id is part of task_struct_rh.
    if (bpf_core_field_exists(((struct task_struct___redhat *)task)->task_struct_rh)) {
        exec_id = BPF_CORE_READ((struct task_struct___redhat *)task, task_struct_rh, self_exec_id);
    } else {
        exec_id = BPF_CORE_READ(task, self_exec_id);
    }

    printk("bpf_prog exec_id: %lx", exec_id);

    return 0;
}

char LICENSE[] SEC("license") = "GPL";
```

At the time of writing, `aya-obj` will generate a ``MissingTargetDefinition`` error on the relocation for ``task_struct_rh`` in the ``BPF_CORE_READ``.

This behavior differ with what [libbpf does](https://github.com/libbpf/libbpf/blob/5b795f7b3028b125bb0ad1db7dc809297b28094f/src/relo_core.c#L1398-L1408).

Missing relocations at load time shouldn't cause an error in `aya-obj` but instead poison related instructions.

If any of those code-path are taken, the verifier will refuse to load the eBPF.




Discussion / hints:
Thank you for the report! I see that you forked aya did you want to take a stab at fixing this, or want me to take a look?
> Thank you for the report! I see that you forked aya did you want to take a stab at fixing this, or want me to take a look?

I did a fix with https://github.com/marysaka/aya/commit/6c5a0845a1985068adb897963cbea21b507fcbca but that basically nullify ``ComputedRelocation::compute_*_relocation`` errors.

I don't know if anyone have a usecase/need for those errors, but if it's fine for you I can PR those changes. 

Fix patch:
diff --git a/aya-obj/src/btf/relocation.rs b/aya-obj/src/btf/relocation.rs
index 3098ab7f7..a352236d3 100644
--- a/aya-obj/src/btf/relocation.rs
+++ b/aya-obj/src/btf/relocation.rs
@@ -14,8 +14,8 @@ use crate::{
         IntEncoding, Struct, Union, MAX_SPEC_LEN,
     },
     generated::{
-        bpf_core_relo, bpf_core_relo_kind::*, bpf_insn, BPF_ALU, BPF_ALU64, BPF_B, BPF_DW, BPF_H,
-        BPF_K, BPF_LD, BPF_LDX, BPF_ST, BPF_STX, BPF_W, BTF_INT_SIGNED,
+        bpf_core_relo, bpf_core_relo_kind::*, bpf_insn, BPF_ALU, BPF_ALU64, BPF_B, BPF_CALL,
+        BPF_DW, BPF_H, BPF_JMP, BPF_K, BPF_LD, BPF_LDX, BPF_ST, BPF_STX, BPF_W, BTF_INT_SIGNED,
     },
     util::HashMap,
     Object, Program, ProgramSection,
@@ -314,13 +314,18 @@ fn relocate_btf_program<'target>(
             // same value, else the relocation is ambiguous and can't be applied
             let conflicts = matches
                 .filter_map(|(cand_name, cand_spec, cand_comp_rel)| {
-                    if cand_spec.bit_offset != target_spec.bit_offset
-                        || cand_comp_rel.target.value != target_comp_rel.target.value
-                    {
-                        Some(cand_name)
-                    } else {
-                        None
+                    if cand_spec.bit_offset != target_spec.bit_offset {
+                        return Some(cand_name);
+                    } else if let (Some(cand_comp_rel_target), Some(target_comp_rel_target)) = (
+                        cand_comp_rel.target.as_ref(),
+                        target_comp_rel.target.as_ref(),
+                    ) {
+                        if cand_comp_rel_target.value != target_comp_rel_target.value {
+                            return Some(cand_name);
+                        }
                     }
+
+                    None
                 })
                 .collect::<Vec<_>>();
             if !conflicts.is_empty() {
@@ -809,7 +814,7 @@ struct Candidate<'a> {
 #[derive(Debug)]
 struct ComputedRelocation {
     local: ComputedRelocationValue,
-    target: ComputedRelocationValue,
+    target: Option<ComputedRelocationValue>,
 }
 
 #[derive(Debug)]
@@ -819,6 +824,14 @@ struct ComputedRelocationValue {
     type_id: Option<u32>,
 }
 
+fn poison_insn(ins: &mut bpf_insn) {
+    ins.code = (BPF_JMP | BPF_CALL) as u8;
+    ins.set_dst_reg(0);
+    ins.set_src_reg(0);
+    ins.off = 0;
+    ins.imm = 0xBAD2310;
+}
+
 impl ComputedRelocation {
     fn new(
         rel: &Relocation,
@@ -830,15 +843,15 @@ impl ComputedRelocation {
             FieldByteOffset | FieldByteSize | FieldExists | FieldSigned | FieldLShift64
             | FieldRShift64 => ComputedRelocation {
                 local: Self::compute_field_relocation(rel, Some(local_spec))?,
-                target: Self::compute_field_relocation(rel, target_spec)?,
+                target: Self::compute_field_relocation(rel, target_spec).ok(),
             },
             TypeIdLocal | TypeIdTarget | TypeExists | TypeSize => ComputedRelocation {
                 local: Self::compute_type_relocation(rel, local_spec, target_spec)?,
-                target: Self::compute_type_relocation(rel, local_spec, target_spec)?,
+                target: Self::compute_type_relocation(rel, local_spec, target_spec).ok(),
             },
             EnumVariantExists | EnumVariantValue => ComputedRelocation {
                 local: Self::compute_enum_relocation(rel, Some(local_spec))?,
-                target: Self::compute_enum_relocation(rel, target_spec)?,
+                target: Self::compute_enum_relocation(rel, target_spec).ok(),
             },
         };
 
@@ -864,9 +877,31 @@ impl ComputedRelocation {
                     relocation_number: rel.number,
                 })?;
 
+        let target = if let Some(target) = self.target.as_ref() {
+            target
+        } else {
+            let is_ld_imm64 = ins.code == (BPF_LD | BPF_DW) as u8;
+
+            poison_insn(ins);
+
+            if is_ld_imm64 {
+                let next_ins = instructions.get_mut(ins_index + 1).ok_or(
+                    RelocationError::InvalidInstructionIndex {
+                        index: (ins_index + 1) * mem::size_of::<bpf_insn>(),
+                        num_instructions,
+                        relocation_number: rel.number,
+                    },
+                )?;
+
+                poison_insn(next_ins);
+            }
+
+            return Ok(());
+        };
+
         let class = (ins.code & 0x07) as u32;
 
-        let target_value = self.target.value;
+        let target_value = target.value;
 
         match class {
             BPF_ALU | BPF_ALU64 => {
@@ -892,9 +927,9 @@ impl ComputedRelocation {
 
                 ins.off = target_value as i16;
 
-                if self.local.size != self.target.size {
+                if self.local.size != target.size {
                     let local_ty = local_btf.type_by_id(self.local.type_id.unwrap())?;
-                    let target_ty = target_btf.type_by_id(self.target.type_id.unwrap())?;
+                    let target_ty = target_btf.type_by_id(target.type_id.unwrap())?;
                     let unsigned = |info: u32| ((info >> 24) & 0x0F) & BTF_INT_SIGNED == 0;
                     use BtfType::*;
                     match (local_ty, target_ty) {
@@ -910,13 +945,13 @@ impl ComputedRelocation {
                                     err_type_name(&local_btf.err_type_name(local_ty)),
                                     self.local.size,
                                     err_type_name(&target_btf.err_type_name(target_ty)),
-                                    self.target.size,
+                                    target.size,
                                 ),
                             })
                         }
                     }
 
-                    let size = match self.target.size {
+                    let size = match target.size {
                         8 => BPF_DW,
                         4 => BPF_W,
                         2 => BPF_H,


Test patch:
diff --git a/test/integration-test/src/tests/btf_relocations.rs b/test/integration-test/src/tests/btf_relocations.rs
index 85291ef42..f8f37c8fa 100644
--- a/test/integration-test/src/tests/btf_relocations.rs
+++ b/test/integration-test/src/tests/btf_relocations.rs
@@ -143,6 +143,35 @@ fn relocate_pointer() {
     assert_eq!(test.run_no_btf().unwrap(), 42);
 }
 
+#[integration_test]
+fn relocate_struct_flavors() {
+    let definition = r#"
+        struct foo {};
+        struct bar { struct foo *f; };
+        struct bar___cafe { struct foo *e; struct foo *f; };
+    "#;
+
+    let relocation_code = r#"
+        __u8 memory[] = {42, 0, 0, 0, 0, 0, 0, 0, 21, 0, 0, 0, 0, 0, 0, 0};
+        struct bar* ptr = (struct bar *) &memory;
+
+        if (__builtin_preserve_field_info((((typeof(struct bar___cafe) *)0)->e), 2)) {
+            value = (__u64) __builtin_preserve_access_index(((struct bar___cafe *)ptr)->e);
+        } else {
+            value = (__u64) __builtin_preserve_access_index(ptr->f);
+        }
+    "#;
+
+    let test_no_flavor = RelocationTest {
+        local_definition: definition,
+        target_btf: definition,
+        relocation_code,
+    }
+    .build()
+    .unwrap();
+    assert_eq!(test_no_flavor.run_no_btf().unwrap(), 42);
+}
+
 /// Utility code for running relocation tests:
 /// - Generates the eBPF program using probided local definition and relocation code
 /// - Generates the BTF from the target btf code