Repository: cilium/ebpf
Pull request #852 (2022-11-11 15:04:28)

btf: check for compatibility first when searching for a CO-RE field
btf: check for compatibility first when searching for a CO-RE field

    Some kernels have both a struct bpf_sk_lookup and a function
    bpf_sk_lookup() in their BTF. When trying to CO-RE relocate struct
    bpf_sk_lookup we attempt to calculate against the function. This should
    generate errImpossibleRelocation but in fact returns this:

        relocate Struct:"bpf_sk_lookup": target Func:"bpf_sk_lookup": unsized type *btf.Func

    This is because in coreFindField we attempt to adjust the offset
    before checking that local and target are actually compatible.

    Call coreAreMembersCompatible before trying to adjust the offset.

    Fixes https://github.com/cilium/ebpf/issues/850

btf: always include coreKind when CO-RE fails

    Include relo.kind in the error string in coreCalculateFixups to
    ensure we always output this important information.

lmb: @SikoJobs can you give this a try and let me know whether it solves your issue please?
SikoJobs: > @SikoJobs can you give this a try and let me know whether it solves your issue please?

It works. Thank you!

Patch:
diff --git a/btf/core.go b/btf/core.go
index f952b65..a0d2c1f 100644
--- a/btf/core.go
+++ b/btf/core.go
@@ -251,7 +251,7 @@ func coreCalculateFixups(relos []*CORERelocation, targetSpec *Spec, targets []Ty
 		for _, relo := range relos {
 			fixup, err := coreCalculateFixup(relo, target, targetID, bo)
 			if err != nil {
-				return nil, fmt.Errorf("target %s: %w", target, err)
+				return nil, fmt.Errorf("target %s: %s: %w", target, relo.kind, err)
 			}
 			if fixup.poison || fixup.isNonExistant() {
 				score++
@@ -320,7 +320,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 	switch relo.kind {
 	case reloTypeIDTarget, reloTypeSize, reloTypeExists:
 		if len(relo.accessor) > 1 || relo.accessor[0] != 0 {
-			return zero, fmt.Errorf("%s: unexpected accessor %v", relo.kind, relo.accessor)
+			return zero, fmt.Errorf("unexpected accessor %v", relo.accessor)
 		}
 
 		err := coreAreTypesCompatible(local, target)
@@ -328,7 +328,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 			return poison()
 		}
 		if err != nil {
-			return zero, fmt.Errorf("relocation %s: %w", relo.kind, err)
+			return zero, err
 		}
 
 		switch relo.kind {
@@ -358,7 +358,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 			return poison()
 		}
 		if err != nil {
-			return zero, fmt.Errorf("relocation %s: %w", relo.kind, err)
+			return zero, err
 		}
 
 		switch relo.kind {
@@ -395,7 +395,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 			return poison()
 		}
 		if err != nil {
-			return zero, fmt.Errorf("target %s: %w", target, err)
+			return zero, err
 		}
 
 		maybeSkipValidation := func(f COREFixup, err error) (COREFixup, error) {
@@ -451,7 +451,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
 		}
 	}
 
-	return zero, fmt.Errorf("relocation %s: %w", relo.kind, ErrNotSupported)
+	return zero, ErrNotSupported
 }
 
 /* coreAccessor contains a path through a struct. It contains at least one index.
@@ -552,6 +552,10 @@ type coreField struct {
 }
 
 func (cf *coreField) adjustOffsetToNthElement(n int) error {
+	if n == 0 {
+		return nil
+	}
+
 	size, err := Sizeof(cf.Type)
 	if err != nil {
 		return err
@@ -608,6 +612,10 @@ func coreFindField(localT Type, localAcc coreAccessor, targetT Type) (coreField,
 	local := coreField{Type: localT}
 	target := coreField{Type: targetT}
 
+	if err := coreAreMembersCompatible(local.Type, target.Type); err != nil {
+		return coreField{}, coreField{}, fmt.Errorf("fields: %w", err)
+	}
+
 	// The first index is used to offset a pointer of the base type like
 	// when accessing an array.
 	if err := local.adjustOffsetToNthElement(localAcc[0]); err != nil {
@@ -618,10 +626,6 @@ func coreFindField(localT Type, localAcc coreAccessor, targetT Type) (coreField,
 		return coreField{}, coreField{}, err
 	}
 
-	if err := coreAreMembersCompatible(local.Type, target.Type); err != nil {
-		return coreField{}, coreField{}, fmt.Errorf("fields: %w", err)
-	}
-
 	var localMaybeFlex, targetMaybeFlex bool
 	for i, acc := range localAcc[1:] {
 		switch localType := local.Type.(type) {


Test patch:
diff --git a/btf/core_test.go b/btf/core_test.go
index f98eb4f..bde875f 100644
--- a/btf/core_test.go
+++ b/btf/core_test.go
@@ -302,6 +302,13 @@ func TestCOREFindField(t *testing.T) {
 			coreAccessor{0, 0},
 			errImpossibleRelocation,
 		},
+		{
+			"unsized type",
+			bStruct, &Func{},
+			// non-zero accessor to force calculating the offset.
+			coreAccessor{1},
+			errImpossibleRelocation,
+		},
 	}
 
 	for _, test := range invalid {