From 7cf14b7c8b5efc4eef19bddd1ad03ee4216091d9 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Mon, 27 Nov 2023 17:15:32 -0800 Subject: [PATCH] Add equality function for BPF instructions. Use it when checking that instructions are equivalent in `rewriteAllJumpsToReturn`. Reported-by: syzbot+637101fb08769773300e@syzkaller.appspotmail.com PiperOrigin-RevId: 585802467 --- pkg/bpf/BUILD | 1 + pkg/bpf/bpf.go | 60 +++++++ pkg/bpf/bpf_test.go | 395 +++++++++++++++++++++++++++++++++++++++++++ pkg/bpf/optimizer.go | 2 +- 4 files changed, 457 insertions(+), 1 deletion(-) create mode 100644 pkg/bpf/bpf_test.go diff --git a/pkg/bpf/BUILD b/pkg/bpf/BUILD index 53862b80f..e9dbfb4f1 100644 --- a/pkg/bpf/BUILD +++ b/pkg/bpf/BUILD @@ -25,6 +25,7 @@ go_test( name = "bpf_test", size = "small", srcs = [ + "bpf_test.go", "decoder_test.go", "interpreter_test.go", "optimizer_test.go", diff --git a/pkg/bpf/bpf.go b/pkg/bpf/bpf.go index dcc69dd4a..60e9c0a62 100644 --- a/pkg/bpf/bpf.go +++ b/pkg/bpf/bpf.go @@ -78,6 +78,7 @@ const ( K = 0x00 // still mode 4 X = 0x08 // mode 0 A = 0x10 // mode 9 + operandMask = K | X | A srcAluJmpMask = 0x08 srcRetMask = 0x18 @@ -149,6 +150,65 @@ func Jump(code uint16, k uint32, jt, jf uint8) Instruction { } } +// Equal returns whether this instruction is equivalent to `other`. +func (ins Instruction) Equal(other Instruction) bool { + if ins.OpCode != other.OpCode { + // If instructions don't have the same opcode, they are not equal. + return false + } + switch ins.OpCode & instructionClassMask { + case Ld, Ldx: + if ins.OpCode&loadModeMask == Len { + // Length instructions are independent of the K register. + return true + } + // Two load instructions are the same if they load from the same offset. + return ins.K == other.K + case St, Stx: + // Two store instructions are the same if they store at the same offset. + return ins.K == other.K + case Alu: + if ins.OpCode == Alu|Neg { + return true // The negation instruction has no operands. + } + if ins.OpCode&operandMask == X { + // If we use X, no need to check anything. + return true + } + if ins.OpCode&operandMask == K { + // If use K, check that it's the same. + return ins.K == other.K + } + // Otherwise, we use the whole instruction. + case Ret: + switch ins.OpCode { + case Ret | A: + // All instructions that return the A register are equivalent. + return true + case Ret | K: + // All instructions that return the same value are equivalent. + return ins.K == other.K + } + case Jmp: + if ins.IsUnconditionalJump() { + // Unconditional jumps to the same offset are equivalent. + return ins.K == other.K + } + if ins.OpCode&operandMask == X { + // If we use X as the operand, check the conditional jump targets only. + return ins.JumpIfTrue == other.JumpIfTrue && ins.JumpIfFalse == other.JumpIfFalse + } + // Otherwise, we use the whole instruction. + case Misc: + if ins.OpCode == Misc|Tax || ins.OpCode == Misc|Txa { + // Swapping X and A, we don't care about the other fields. + return true + } + } + // All other instructions need full bit-for-bit comparison. + return ins == other +} + // IsReturn returns true if `ins` is a return instruction. func (ins Instruction) IsReturn() bool { return ins.OpCode&instructionClassMask == Ret diff --git a/pkg/bpf/bpf_test.go b/pkg/bpf/bpf_test.go new file mode 100644 index 000000000..e5e066c29 --- /dev/null +++ b/pkg/bpf/bpf_test.go @@ -0,0 +1,395 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bpf + +import ( + "testing" +) + +func TestEqual(t *testing.T) { + for _, test := range []struct { + name string + a, b Instruction + want bool + }{ + { + name: "empty instructions", + want: true, + }, + { + name: "two different invalid instructions", + a: Instruction{ + OpCode: 0xffff, + }, + b: Instruction{ + OpCode: 0xfffe, + }, + want: false, + }, + { + name: "two loads from different offsets", + a: Instruction{ + OpCode: Ld | Imm | W, + K: 1234, + }, + b: Instruction{ + OpCode: Ld | Imm | W, + K: 5678, + }, + want: false, + }, + { + name: "two loads from same offsets but different source", + a: Instruction{ + OpCode: Ld | Mem | W, + K: 1234, + }, + b: Instruction{ + OpCode: Ld | Imm | W, + K: 1234, + }, + want: false, + }, + { + name: "two loads from same offsets but different conditional jump fields", + a: Instruction{ + OpCode: Ld | Mem | W, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Ld | Mem | W, + K: 1234, + }, + want: true, + }, + { + name: "two length loads", + a: Instruction{ + OpCode: Ld | Len | W, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Ld | Len | W, + K: 5678, + JumpIfTrue: 99, + }, + want: true, + }, + { + name: "two length loads in different registers", + a: Instruction{ + OpCode: Ld | Len | W, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Ldx | Len | W, + K: 1234, + JumpIfTrue: 12, + }, + want: false, + }, + { + name: "two stores at different offsets", + a: Instruction{ + OpCode: St, + K: 1234, + }, + b: Instruction{ + OpCode: St, + K: 5678, + }, + want: false, + }, + { + name: "two stores at same offsets but different source", + a: Instruction{ + OpCode: St, + K: 1234, + }, + b: Instruction{ + OpCode: Stx, + K: 1234, + }, + want: false, + }, + { + name: "two stores at same offsets but different conditional jump fields", + a: Instruction{ + OpCode: St, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: St, + K: 1234, + }, + want: true, + }, + { + name: "two negation ALUs with different other fields", + a: Instruction{ + OpCode: Alu | Neg, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Alu | Neg, + K: 5678, + JumpIfTrue: 34, + }, + want: true, + }, + { + name: "two 'add K' ALUs with different K", + a: Instruction{ + OpCode: Alu | Add | K, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Alu | Add | K, + K: 5678, + JumpIfTrue: 34, + }, + want: false, + }, + { + name: "two 'add X' ALUs with different K", + a: Instruction{ + OpCode: Alu | Add | X, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Alu | Add | X, + K: 5678, + JumpIfTrue: 34, + }, + want: true, + }, + { + name: "two 'return A' instructions with different K", + a: Instruction{ + OpCode: Ret | A, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Ret | A, + K: 5678, + JumpIfTrue: 34, + }, + want: true, + }, + { + name: "two 'return K' instructions with same K", + a: Instruction{ + OpCode: Ret | K, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Ret | K, + K: 1234, + JumpIfTrue: 34, + }, + want: true, + }, + { + name: "two 'return K' instructions with different K", + a: Instruction{ + OpCode: Ret | K, + K: 1234, + }, + b: Instruction{ + OpCode: Ret | K, + K: 5678, + }, + want: false, + }, + { + name: "two unconditional jumps with different K", + a: Instruction{ + OpCode: Jmp | Ja, + K: 1234, + }, + b: Instruction{ + OpCode: Jmp | Ja, + K: 5678, + }, + want: false, + }, + { + name: "two unconditional jumps with same K", + a: Instruction{ + OpCode: Jmp | Ja, + K: 1234, + JumpIfTrue: 12, + }, + b: Instruction{ + OpCode: Jmp | Ja, + K: 1234, + JumpIfTrue: 34, + }, + want: true, + }, + { + name: "two conditional jumps using K with same K", + a: Instruction{ + OpCode: Jmp | Jgt | K, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Jmp | Jgt | K, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + want: true, + }, + { + name: "two conditional jumps using K with different K", + a: Instruction{ + OpCode: Jmp | Jgt | K, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Jmp | Jgt | K, + K: 5678, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + want: false, + }, + { + name: "two conditional jumps using X with different K", + a: Instruction{ + OpCode: Jmp | Jgt | X, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Jmp | Jgt | X, + K: 5678, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + want: true, + }, + { + name: "two conditional jumps with different 'true' jump target", + a: Instruction{ + OpCode: Jmp | Jgt | X, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Jmp | Jgt | X, + K: 1234, + JumpIfTrue: 99, + JumpIfFalse: 21, + }, + want: false, + }, + { + name: "two conditional jumps with different 'false' jump target", + a: Instruction{ + OpCode: Jmp | Jgt | X, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Jmp | Jgt | X, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 99, + }, + want: false, + }, + { + name: "two txa instructions", + a: Instruction{ + OpCode: Misc | Txa, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Misc | Txa, + K: 5678, + JumpIfTrue: 34, + JumpIfFalse: 42, + }, + want: true, + }, + { + name: "two tax instructions", + a: Instruction{ + OpCode: Misc | Tax, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Misc | Tax, + K: 5678, + JumpIfTrue: 34, + JumpIfFalse: 42, + }, + want: true, + }, + { + name: "two different misc instructions", + a: Instruction{ + OpCode: Misc | Txa, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + b: Instruction{ + OpCode: Misc | Tax, + K: 1234, + JumpIfTrue: 12, + JumpIfFalse: 21, + }, + want: false, + }, + } { + t.Run(test.name, func(t *testing.T) { + as := test.a.String() + bs := test.b.String() + got := test.a.Equal(test.b) + if got != test.want { + t.Errorf("%v.Equal(%v) = %v, want %v", as, bs, got, test.want) + } + if reverse := test.b.Equal(test.a); reverse != got { + t.Errorf("%v.Equal(%v) [%v] != %v.Equal(%v) [%v]", as, bs, got, bs, as, reverse) + } + if !t.Failed() && !got && test.a == test.b { + t.Errorf("%v == %v, yet %v.Equal(%v) is false", as, bs, as, bs) + } + }) + } +} diff --git a/pkg/bpf/optimizer.go b/pkg/bpf/optimizer.go index a0974a075..c80dff14d 100644 --- a/pkg/bpf/optimizer.go +++ b/pkg/bpf/optimizer.go @@ -294,7 +294,7 @@ func rewriteAllJumpsToReturn(insns []Instruction, fromPC, toPC int) bool { if !toIns.IsReturn() { panic(fmt.Sprintf("attempted to rewrite jumps to {pc=%d: %v} which is not a return instruction", toPC, toIns)) } - if fromIns != toIns { + if !fromIns.Equal(toIns) { panic(fmt.Sprintf("attempted to rewrite jump target to a different return instruction: from={pc=%d: %v}, to={pc=%d: %v}", fromPC, fromIns, toPC, toIns)) } // Scan once, and populate `rewriteOps` as a list of functions that should