From 1b80983ef8df60bab6f010a5a64238f01462601f Mon Sep 17 00:00:00 2001 From: Zan <39847495+CreditWorthy@users.noreply.github.com> Date: Wed, 11 Mar 2026 13:30:52 -0500 Subject: [PATCH 1/2] Fix signed div overflow, call checks & add tests Avoid mutating destination registers when signed division/remainder overflows by only performing the operation in the non-overflow branch; preserve semantics for divide-by-zero errors. Improve call handling: check stack push result before changing pc, validate Callx target is inside the text segment and return a dedicated ExcCallOutsideTextSegment error on out-of-bounds. Add Exception.R to capture registers and wrap error with %w for better error inspection. Update opcode mappings for Sdiv variants and extend tests (pkg/sbpf/interpreter_test.go) to cover call depth/out-of-bounds, signed divide overflow and divide-by-zero cases. --- pkg/sbpf/interpreter.go | 49 +++--- pkg/sbpf/interpreter_test.go | 278 +++++++++++++++++++++++++++++++++++ pkg/sbpf/opcode_test.go | 8 +- pkg/sbpf/vm.go | 2 + 4 files changed, 314 insertions(+), 23 deletions(-) create mode 100644 pkg/sbpf/interpreter_test.go diff --git a/pkg/sbpf/interpreter.go b/pkg/sbpf/interpreter.go index dc26d2ea..69187e8d 100644 --- a/pkg/sbpf/interpreter.go +++ b/pkg/sbpf/interpreter.go @@ -519,8 +519,9 @@ mainLoop: } if int32(r[ins.Dst()]) == math.MinInt32 && ins.Imm() == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) / ins.Imm())) } - r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) / ins.Imm())) pc++ case OpSdiv32Reg: if !ip.sbpfVersion.EnablePqr() { @@ -530,8 +531,9 @@ mainLoop: if src := int32(r[ins.Src()]); src != 0 { if int32(r[ins.Dst()]) == math.MinInt32 && src == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) / src)) } - r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) / src)) } else { err = ExcDivideByZero } @@ -543,8 +545,9 @@ mainLoop: } if int64(r[ins.Dst()]) == math.MinInt64 && ins.Imm() == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(int64(r[ins.Dst()]) / int64(ins.Imm())) } - r[ins.Dst()] = uint64(int64(r[ins.Dst()]) / int64(ins.Imm())) pc++ case OpSdiv64Reg: if !ip.sbpfVersion.EnablePqr() { @@ -554,8 +557,9 @@ mainLoop: if src := int64(r[ins.Src()]); src != 0 { if int64(r[ins.Dst()]) == math.MinInt64 && src == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(int64(r[ins.Dst()]) / src) } - r[ins.Dst()] = uint64(int64(r[ins.Dst()]) / src) } else { err = ExcDivideByZero } @@ -567,8 +571,9 @@ mainLoop: } if int32(r[ins.Dst()]) == math.MinInt32 && ins.Imm() == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) % ins.Imm())) } - r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) % ins.Imm())) pc++ case OpSrem32Reg: if !ip.sbpfVersion.EnablePqr() { @@ -578,8 +583,9 @@ mainLoop: if src := int32(r[ins.Src()]); src != 0 { if int32(r[ins.Dst()]) == math.MinInt32 && src == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) % int32(r[ins.Src()]))) } - r[ins.Dst()] = uint64(uint32(int32(r[ins.Dst()]) % int32(r[ins.Src()]))) } else { err = ExcDivideByZero } @@ -591,8 +597,9 @@ mainLoop: } if int64(r[ins.Dst()]) == math.MinInt64 && ins.Imm() == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(int64(r[ins.Dst()]) % int64(ins.Imm())) } - r[ins.Dst()] = uint64(int64(r[ins.Dst()]) % int64(ins.Imm())) pc++ case OpSrem64Reg: if !ip.sbpfVersion.EnablePqr() { @@ -602,8 +609,9 @@ mainLoop: if src := int64(r[ins.Src()]); src != 0 { if int64(r[ins.Dst()]) == math.MinInt64 && src == -1 { err = ExcDivideOverflow + } else { + r[ins.Dst()] = uint64(int64(r[ins.Dst()]) % int64(r[ins.Src()])) } - r[ins.Dst()] = uint64(int64(r[ins.Dst()]) % int64(r[ins.Src()])) } else { err = ExcDivideByZero } @@ -916,11 +924,12 @@ mainLoop: r[0], err = sc.Invoke(ip, r[1], r[2], r[3], r[4], r[5]) pc++ } else if target, ok := ip.funcs[ins.Uimm()]; ok { - ok = ip.stack.Push(r[:], pc+1) - if !ok { + pushed := ip.stack.Push(r[:], pc+1) + if !pushed { err = ExcCallDepth + } else { + pc = target } - pc = target } else { err = ExcCallDest{ins.Uimm()} } @@ -933,15 +942,16 @@ mainLoop: } target &= ^(uint64(0x7)) - var ok bool - ok = ip.stack.Push(r[:], pc+1) - if !ok { + pushed := ip.stack.Push(r[:], pc+1) + if !pushed { err = ExcCallDepth + } else { + if target < ip.textVA || target >= VaddrStack || target >= ip.textVA+uint64(len(ip.text)*8) { + err = ExcCallOutsideTextSegment + } else { + pc = int64((target - ip.textVA) / 8) + } } - if target < ip.textVA || target >= VaddrStack || target >= ip.textVA+uint64(len(ip.text)*8) { - err = NewExcBadAccess(target, 8, false, "jump out-of-bounds") - } - pc = int64((target - ip.textVA) / 8) case OpExit: var ok bool pc, ok = ip.stack.Pop(r[:]) @@ -962,7 +972,8 @@ mainLoop: if err != nil { exc := &Exception{ PC: pc, - Detail: fmt.Errorf("tx: %s, programId: %s - %s:", ip.txSignature, ip.programId, err), + R: r, + Detail: fmt.Errorf("tx: %s, programId: %s - %w:", ip.txSignature, ip.programId, err), } if IsLongIns(ins.Op()) { exc.PC-- // fix reported PC diff --git a/pkg/sbpf/interpreter_test.go b/pkg/sbpf/interpreter_test.go new file mode 100644 index 00000000..fce59657 --- /dev/null +++ b/pkg/sbpf/interpreter_test.go @@ -0,0 +1,278 @@ +package sbpf + +import ( + "errors" + "math" + "testing" + + "github.com/Overclock-Validator/mithril/pkg/cu" + "github.com/Overclock-Validator/mithril/pkg/sbpf/sbpfver" + "github.com/stretchr/testify/require" +) + +func makeSlot(op uint8, dst uint8, src uint8, off int16, imm int32) Slot { + return Slot(uint64(op) | + uint64(dst&0xF)<<8 | + uint64(src&0xF)<<12 | + uint64(uint16(off))<<16 | + uint64(uint32(imm))<<32) +} + +func lddw(dst uint8, val uint64) (Slot, Slot) { + lo := int32(val) + hi := int32(val >> 32) + return makeSlot(OpLddw, dst, 0, 0, lo), Slot(uint64(uint32(hi)) << 32) +} + +func noSyscalls(_ uint32) (Syscall, bool) { return nil, false } + +func newTestInterpreter(text []Slot, funcs map[uint32]int64, ver sbpfver.SbpfVersion) *Interpreter { + p := &Program{ + Text: text, + TextVA: VaddrProgram, + Entrypoint: 0, + Funcs: funcs, + SbpfVersion: ver, + } + meter := cu.NewComputeMeter(1000) + opts := &VMOpts{ + HeapMax: 256 * 1024, + Syscalls: noSyscalls, + ComputeMeter: &meter, + } + return NewInterpreter(p, opts) +} + +func getException(t *testing.T, err error) *Exception { + t.Helper() + var exc *Exception + require.True(t, errors.As(err, &exc), "expected *Exception, got %T: %v", err, err) + return exc +} + +func TestCallErrors(t *testing.T) { + callxAddr := VaddrProgram + 2*8 + callxLo, callxHi := lddw(9, callxAddr) + badLo, badHi := lddw(9, 0xDEAD_0000_0000) + + tests := []struct { + name string + text []Slot + funcs map[uint32]int64 + ver sbpfver.SbpfVersion + wantErr error + wantPC int64 + }{ + { + name: "OpCall/CallDepthExceeded", + text: []Slot{ + makeSlot(OpCall, 0, 0, 0, 1), + makeSlot(OpCall, 0, 0, 0, 1), + makeSlot(OpExit, 0, 0, 0, 0), + }, + funcs: map[uint32]int64{1: 1}, + ver: sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV0}, + wantErr: ExcCallDepth, + wantPC: 1, + }, + { + name: "OpCallx/CallDepthExceeded", + text: []Slot{ + callxLo, + callxHi, + makeSlot(OpCallx, 0, 0, 0, 9), + makeSlot(OpExit, 0, 0, 0, 0), + }, + funcs: map[uint32]int64{}, + ver: sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV0}, + wantErr: ExcCallDepth, + wantPC: 2, + }, + { + name: "OpCallx/OutOfBoundsTarget", + text: []Slot{ + badLo, + badHi, + makeSlot(OpCallx, 0, 0, 0, 9), + makeSlot(OpExit, 0, 0, 0, 0), + }, + funcs: map[uint32]int64{}, + ver: sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV0}, + wantErr: ExcCallOutsideTextSegment, + wantPC: 2, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ip := newTestInterpreter(tc.text, tc.funcs, tc.ver) + defer ip.Finish() + + _, _, err := ip.Run() + exc := getException(t, err) + require.ErrorIs(t, exc.Detail, tc.wantErr) + require.Equal(t, tc.wantPC, exc.PC) + }) + } +} + +func TestSignedDivOverflow(t *testing.T) { + v2 := sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV2} + + tests := []struct { + name string + text []Slot + wantR0 uint64 + }{ + { + name: "OpSdiv32Imm", + text: []Slot{ + makeSlot(OpMov32Imm, 0, 0, 0, math.MinInt32), + makeSlot(OpSdiv32Imm, 0, 0, 0, -1), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x80000000, + }, + { + name: "OpSdiv32Reg", + text: []Slot{ + makeSlot(OpMov32Imm, 0, 0, 0, math.MinInt32), + makeSlot(OpMov32Imm, 1, 0, 0, -1), + makeSlot(OpSdiv32Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x80000000, + }, + { + name: "OpSdiv64Imm", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 1), + makeSlot(OpLsh64Imm, 0, 0, 0, 63), + makeSlot(OpSdiv64Imm, 0, 0, 0, -1), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x8000000000000000, + }, + { + name: "OpSdiv64Reg", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 1), + makeSlot(OpLsh64Imm, 0, 0, 0, 63), + makeSlot(OpMov64Imm, 1, 0, 0, -1), + makeSlot(OpSdiv64Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x8000000000000000, + }, + { + name: "OpSrem32Imm", + text: []Slot{ + makeSlot(OpMov32Imm, 0, 0, 0, math.MinInt32), + makeSlot(OpSrem32Imm, 0, 0, 0, -1), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x80000000, + }, + { + name: "OpSrem32Reg", + text: []Slot{ + makeSlot(OpMov32Imm, 0, 0, 0, math.MinInt32), + makeSlot(OpMov32Imm, 1, 0, 0, -1), + makeSlot(OpSrem32Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x80000000, + }, + { + name: "OpSrem64Imm", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 1), + makeSlot(OpLsh64Imm, 0, 0, 0, 63), + makeSlot(OpSrem64Imm, 0, 0, 0, -1), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x8000000000000000, + }, + { + name: "OpSrem64Reg", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 1), + makeSlot(OpLsh64Imm, 0, 0, 0, 63), + makeSlot(OpMov64Imm, 1, 0, 0, -1), + makeSlot(OpSrem64Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 0x8000000000000000, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ip := newTestInterpreter(tc.text, map[uint32]int64{}, v2) + defer ip.Finish() + + _, _, err := ip.Run() + exc := getException(t, err) + require.ErrorIs(t, exc.Detail, ExcDivideOverflow) + require.Equal(t, tc.wantR0, exc.R[0], "r0 should not be modified on overflow") + }) + } +} + +func TestSignedDivByZero(t *testing.T) { + v2 := sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV2} + + tests := []struct { + name string + text []Slot + }{ + { + name: "OpSdiv32Reg", + text: []Slot{ + makeSlot(OpMov32Imm, 0, 0, 0, 42), + makeSlot(OpMov32Imm, 1, 0, 0, 0), + makeSlot(OpSdiv32Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + }, + { + name: "OpSdiv64Reg", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 42), + makeSlot(OpMov64Imm, 1, 0, 0, 0), + makeSlot(OpSdiv64Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + }, + { + name: "OpSrem32Reg", + text: []Slot{ + makeSlot(OpMov32Imm, 0, 0, 0, 42), + makeSlot(OpMov32Imm, 1, 0, 0, 0), + makeSlot(OpSrem32Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + }, + { + name: "OpSrem64Reg", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 42), + makeSlot(OpMov64Imm, 1, 0, 0, 0), + makeSlot(OpSrem64Reg, 0, 1, 0, 0), + makeSlot(OpExit, 0, 0, 0, 0), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ip := newTestInterpreter(tc.text, map[uint32]int64{}, v2) + defer ip.Finish() + + _, _, err := ip.Run() + exc := getException(t, err) + require.ErrorIs(t, exc.Detail, ExcDivideByZero) + require.Equal(t, uint64(42), exc.R[0], "r0 should not be modified on divide by zero") + }) + } +} diff --git a/pkg/sbpf/opcode_test.go b/pkg/sbpf/opcode_test.go index 8404f6f6..4cc1855c 100644 --- a/pkg/sbpf/opcode_test.go +++ b/pkg/sbpf/opcode_test.go @@ -80,10 +80,10 @@ func TestOpcodes(t *testing.T) { {0xcf, OpArsh64Reg}, {0xd4, OpLe}, {0xdc, OpBe}, - {0xe4, OpSdiv32Imm}, - {0xe7, OpSdiv64Imm}, - {0xec, OpSdiv32Reg}, - {0xef, OpSdiv64Reg}, + {0xc6, OpSdiv32Imm}, + {0xd6, OpSdiv64Imm}, + {0xce, OpSdiv32Reg}, + {0xde, OpSdiv64Reg}, {0x05, OpJa}, {0x15, OpJeqImm}, diff --git a/pkg/sbpf/vm.go b/pkg/sbpf/vm.go index 0c78bac6..b78ddecf 100644 --- a/pkg/sbpf/vm.go +++ b/pkg/sbpf/vm.go @@ -57,6 +57,7 @@ type VMOpts struct { type Exception struct { PC int64 + R [11]uint64 Detail error } @@ -77,6 +78,7 @@ var ( ExcInvalidInstr = errors.New("invalid instruction - feature not enabled") ExcUnsupportedInstruction = errors.New("unsupported BPF instruction") + ExcCallOutsideTextSegment = errors.New("call outside text segment") ) type ExcBadAccess struct { From ee27317537a1c960434359316443b1532d6be7e5 Mon Sep 17 00:00:00 2001 From: Zan <39847495+CreditWorthy@users.noreply.github.com> Date: Wed, 11 Mar 2026 23:39:44 -0500 Subject: [PATCH 2/2] fix(sbpf): preserve PC on top-level OpExit stack pop (#125) stack.Pop() returns (0, false) when the call stack is empty, which overwrote pc to 0 on top-level exits. Save pc before Pop() and restore it when the stack is empty so the final register state reports the correct exit instruction address. Adds regression test TestOpExitTopLevel. --- pkg/sbpf/interpreter.go | 2 ++ pkg/sbpf/interpreter_test.go | 41 ++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/pkg/sbpf/interpreter.go b/pkg/sbpf/interpreter.go index 69187e8d..0154261a 100644 --- a/pkg/sbpf/interpreter.go +++ b/pkg/sbpf/interpreter.go @@ -953,10 +953,12 @@ mainLoop: } } case OpExit: + exitPC := pc var ok bool pc, ok = ip.stack.Pop(r[:]) if !ok { ret = r[0] + pc = exitPC break mainLoop } default: diff --git a/pkg/sbpf/interpreter_test.go b/pkg/sbpf/interpreter_test.go index fce59657..aedc03a7 100644 --- a/pkg/sbpf/interpreter_test.go +++ b/pkg/sbpf/interpreter_test.go @@ -116,6 +116,47 @@ func TestCallErrors(t *testing.T) { } } +func TestOpExitTopLevel(t *testing.T) { + v0 := sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV0} + + tests := []struct { + name string + text []Slot + wantR0 uint64 + }{ + { + name: "SingleExit", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 42), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 42, + }, + { + name: "ExitAtPC4", + text: []Slot{ + makeSlot(OpMov64Imm, 0, 0, 0, 1), + makeSlot(OpAdd64Imm, 0, 0, 0, 2), + makeSlot(OpAdd64Imm, 0, 0, 0, 3), + makeSlot(OpAdd64Imm, 0, 0, 0, 4), + makeSlot(OpExit, 0, 0, 0, 0), + }, + wantR0: 10, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ip := newTestInterpreter(tc.text, map[uint32]int64{}, v0) + defer ip.Finish() + + ret, _, err := ip.Run() + require.NoError(t, err) + require.Equal(t, tc.wantR0, ret) + }) + } +} + func TestSignedDivOverflow(t *testing.T) { v2 := sbpfver.SbpfVersion{Version: sbpfver.SbpfVersionV2}