asm: allow negative constants for builtin function calls

lmb · lmb · commit dab673cc3076 · 2025-06-24T12:03:47.000+01:00
The work to encode a platform into various constant types made it so that decoding a call to a builtin helper with a negative value fails with decoding instructions for section <sectionname>: offset <offset>: invalid constant 0xffffffff for a BPF instruction of "call -1". This is because we can't represent -1 as a tagged platform constant. Allow negative constants by not transforming them into a platform constant at all. Adjust the platform tag size so that we never generate a platform constant with the high bit set. This avoids confusing it with a negative number when reinterpreting it as a signed number and ensures that trying to marshal such an instruction gives an error. Fixes: #1797 Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
diff --git a/asm/instruction.go b/asm/instruction.go
@@ -67,11 +67,17 @@ func (ins *Instruction) Unmarshal(r io.Reader, bo binary.ByteOrder, platform str
 	ins.Constant = int64(int32(bo.Uint32(data[4:8])))
 
 	if ins.IsBuiltinCall() {
-		fn, err := BuiltinFuncForPlatform(platform, uint32(ins.Constant))
-		if err != nil {
-			return err
+		if ins.Constant >= 0 {
+			// Leave negative constants from the instruction stream
+			// unchanged. These are sometimes used as placeholders for later
+			// patching.
+			// This relies on not having a valid platform tag with a high bit set.
+			fn, err := BuiltinFuncForPlatform(platform, uint32(ins.Constant))
+			if err != nil {
+				return err
+			}
+			ins.Constant = int64(fn)
 		}
-		ins.Constant = int64(fn)
 	} else if ins.OpCode.Class().IsALU() {
 		switch ins.OpCode.ALUOp() {
 		case Div:
diff --git a/asm/instruction_test.go b/asm/instruction_test.go
@@ -180,6 +180,16 @@ func TestInstructionWithMetadata(t *testing.T) {
 	}
 }
 
+func TestReadCallToNegativeOne(t *testing.T) {
+	raw := []byte{
+		0x85, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff,
+	}
+	var ins Instruction
+	err := ins.Unmarshal(bytes.NewReader(raw), binary.LittleEndian, platform.Linux)
+	qt.Assert(t, qt.IsNil(err))
+	qt.Assert(t, qt.Equals(ins.Constant, -1))
+}
+
 // You can use format flags to change the way an eBPF
 // program is stringified.
 func ExampleInstructions_Format() {
diff --git a/internal/platform/constants.go b/internal/platform/constants.go
@@ -11,7 +11,7 @@ const (
 )
 
 const (
-	platformMax   = 0xf
+	platformMax   = 1<<3 - 1 // most not exceed 3 bits to avoid setting the high bit
 	platformShift = 28
 	platformMask  = platformMax << platformShift
 )
diff --git a/internal/platform/constants_test.go b/internal/platform/constants_test.go
@@ -7,7 +7,7 @@ import (
 )
 
 func TestConstant(t *testing.T) {
-	const maxConstant = ^uint32(platformMask)
+	const maxConstant = uint32(1<<platformShift - 1)
 	for _, plat := range []string{
 		Linux,
 		Windows,

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ const (`
`11`	`11`	`)`
`12`	`12`
`13`	`13`	`const (`
`14`		`- platformMax = 0xf`
	`14`	`+ platformMax = 1<<3 - 1 // most not exceed 3 bits to avoid setting the high bit`
`15`	`15`	`platformShift = 28`
`16`	`16`	`platformMask = platformMax << platformShift`
`17`	`17`	`)`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ import (`
`7`	`7`	`)`
`8`	`8`
`9`	`9`	`func TestConstant(t *testing.T) {`
`10`		`- const maxConstant = ^uint32(platformMask)`
	`10`	`+ const maxConstant = uint32(1<<platformShift - 1)`
`11`	`11`	`for _, plat := range []string{`
`12`	`12`	`Linux,`
`13`	`13`	`Windows,`