Repository: cilium/ebpf
Pull request #1768 (2025-04-30 09:26:21)

fix: return InvalidOpCode for StoreImm with DWord size
fixed: #1767


Patch:
diff --git a/asm/load_store.go b/asm/load_store.go
index 30917b8..29571a7 100644
--- a/asm/load_store.go
+++ b/asm/load_store.go
@@ -313,6 +313,10 @@ func StoreImmOp(size Size) OpCode {
 
 // StoreImm emits `*(size *)(dst + offset) = value`.
 func StoreImm(dst Register, offset int16, value int64, size Size) Instruction {
+	if size == DWord {
+		return Instruction{OpCode: InvalidOpCode}
+	}
+
 	return Instruction{
 		OpCode:   StoreImmOp(size),
 		Dst:      dst,
diff --git a/btf/btf.go b/btf/btf.go
index 8112b6c..d26931a 100644
--- a/btf/btf.go
+++ b/btf/btf.go
@@ -346,7 +346,10 @@ func fixupDatasecLayout(ds *Datasec) error {
 	return nil
 }
 
-// Copy creates a copy of Spec.
+// Copy a Spec.
+//
+// All contained types are duplicated while preserving any modifications made
+// to them.
 func (s *Spec) Copy() *Spec {
 	if s == nil {
 		return nil
diff --git a/btf/unmarshal.go b/btf/unmarshal.go
index a55d79d..8704140 100644
--- a/btf/unmarshal.go
+++ b/btf/unmarshal.go
@@ -47,7 +47,6 @@ func newDecoder(raw []byte, bo binary.ByteOrder, strings *stringTable, base *dec
 			return nil, fmt.Errorf("can't use split BTF as base")
 		}
 
-		base = base.Copy()
 		firstTypeID = TypeID(len(base.offsets))
 	}
 
@@ -166,23 +165,35 @@ func allBtfTypeOffsets(buf []byte, bo binary.ByteOrder, header *btfType) iter.Se
 	}
 }
 
+// Copy performs a deep copy of a decoder and its base.
 func (d *decoder) Copy() *decoder {
 	if d == nil {
 		return nil
 	}
 
+	return d.copy(nil)
+}
+
+func (d *decoder) copy(copiedTypes map[Type]Type) *decoder {
+	if d == nil {
+		return nil
+	}
+
 	d.mu.Lock()
 	defer d.mu.Unlock()
 
+	if copiedTypes == nil {
+		copiedTypes = make(map[Type]Type, len(d.types))
+	}
+
 	types := make(map[TypeID]Type, len(d.types))
-	copiedTypes := make(map[Type]Type, len(d.types))
 	typeIDs := make(map[Type]TypeID, len(d.typeIDs))
 	for id, typ := range d.types {
 		types[id] = copyType(typ, d.typeIDs, copiedTypes, typeIDs)
 	}
 
 	return &decoder{
-		d.base,
+		d.base.copy(copiedTypes),
 		d.byteOrder,
 		d.raw,
 		d.strings,


Test patch:
diff --git a/btf/btf_test.go b/btf/btf_test.go
index 68f57f3..594533e 100644
--- a/btf/btf_test.go
+++ b/btf/btf_test.go
@@ -447,50 +447,59 @@ func TestLoadSplitSpecFromReader(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	typ, err := splitSpec.AnyTypeByName("bpf_testmod_init")
-	if err != nil {
-		t.Fatal(err)
-	}
-	typeID, err := splitSpec.TypeID(typ)
-	if err != nil {
-		t.Fatal(err)
-	}
+	var fnType *Func
+	qt.Assert(t, qt.IsNil(splitSpec.TypeByName("bpf_testmod_init", &fnType)))
+	typeID, err := splitSpec.TypeID(fnType)
+	qt.Assert(t, qt.IsNil(err))
 
 	typeByID, err := splitSpec.TypeByID(typeID)
 	qt.Assert(t, qt.IsNil(err))
-	qt.Assert(t, qt.Equals(typeByID, typ))
+	qt.Assert(t, qt.Equals(typeByID, Type(fnType)))
 
-	fnType := typ.(*Func)
 	fnProto := fnType.Type.(*FuncProto)
+	_, err = spec.TypeID(fnProto)
+	qt.Assert(t, qt.IsNil(err), qt.Commentf("FuncProto should be in base"))
 
 	// 'int' is defined in the base BTF...
 	intType, err := spec.AnyTypeByName("int")
-	if err != nil {
-		t.Fatal(err)
-	}
+	qt.Assert(t, qt.IsNil(err))
+
 	// ... but not in the split BTF
 	_, err = splitSpec.AnyTypeByName("int")
-	if err == nil {
-		t.Fatal("'int' is not supposed to be found in the split BTF")
-	}
+	qt.Assert(t, qt.ErrorIs(err, ErrNotFound))
+
+	qt.Assert(t, qt.Equals(fnProto.Return, intType),
+		qt.Commentf("types found in base of split spec should be reused"))
 
-	qt.Assert(t, qt.Not(qt.Equals(fnProto.Return, intType)),
-		qt.Commentf("types found in base of split spec should be copies"))
+	fnProto.Params = []FuncParam{{"a", &Pointer{(*Void)(nil)}}}
 
-	// Check that copied split-BTF's spec has correct type indexing
+	// The behaviour of copying a split spec is quite subtle. When initially
+	// creating a split spec, types in the split base are shared. This allows
+	// amortising the cost of decoding vmlinux.
+	//
+	// However, we currently define copying a spec to be like forking a process:
+	// in-memory changes to types are preserved. After the copy finished we have
+	// two fully independent states.
+	//
+	// For split BTF this means that we also need to copy the base and ensure
+	// that future references to a modified type work correctly.
 	splitSpecCopy := splitSpec.Copy()
-	copyType, err := splitSpecCopy.AnyTypeByName("bpf_testmod_init")
-	if err != nil {
-		t.Fatal(err)
-	}
-	copyTypeID, err := splitSpecCopy.TypeID(copyType)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if copyTypeID != typeID {
-		t.Fatalf("'bpf_testmod_init` type ID (%d) does not match copied spec's (%d)",
-			typeID, copyTypeID)
-	}
+
+	var fnCopyType *Func
+	qt.Assert(t, qt.IsNil(splitSpecCopy.TypeByName("bpf_testmod_init", &fnCopyType)))
+	qt.Assert(t, testutils.IsDeepCopy(fnCopyType, fnType))
+
+	// Pull out a second type which refers to "int" in the base, but which hasn't
+	// been inflated yet. This forces inflating int from the base.
+	var str *Struct
+	qt.Assert(t, qt.IsNil(splitSpecCopy.TypeByName("bpf_testmod_struct_arg_1", &str)))
+
+	// Ensure that the int types are indeed the same.
+	qt.Assert(t, qt.Equals(str.Members[0].Type, fnCopyType.Type.(*FuncProto).Return))
+
+	copyTypeID, err := splitSpecCopy.TypeID(fnCopyType)
+	qt.Assert(t, qt.IsNil(err))
+	qt.Assert(t, qt.Equals(copyTypeID, typeID), qt.Commentf("ID of copied type must match"))
 }
 
 func TestFixupDatasecLayout(t *testing.T) {