bpf: fix VM out of bounds LoadMemShift check

The bpf VM did not correctly check the bounds of LoadMemShift
instructions, as it used a size of 0 instead of the correct 1.

A LoadMemShift instruction 1 past the end of the input resulted in a
runtime panic:

    panic(0x5c1d40, 0x7cec00)
            /usr/local/go/src/runtime/panic.go:522 +0x1b5
    golang.org/x/net/bpf.loadMemShift(...)
            /home/afabre/go/pkg/mod/golang.org/x/net@v0.0.0-20190603091049-60506f45cf65/bpf/vm_instructions.go:137
    golang.org/x/net/bpf.(*VM).Run(0xc00000ec40, 0xc0000173c8, 0x2, 0x8, 0x2, 0xc0000173c8, 0x0)
            /home/afabre/go/pkg/mod/golang.org/x/net@v0.0.0-20190603091049-60506f45cf65/bpf/vm.go:131 +0xb0a

Fix this, and rework the out of bounds tests for load instructions to:

* Use an offset one past the end of the input, to catch this

* Use a filter that returns 1, to catch cases were the out of bounds
load does not cause a panic, but does not cause the VM to return 0.

Change-Id: I1e68886915207a34f59765805f907f36dc031f70
Reviewed-on: https://go-review.googlesource.com/c/net/+/180979
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>

bpf/vm_instructions.go

@@ -129,7 +129,8 @@ func loadIndirect(ins LoadIndirect, in []byte, regX uint32) (uint32, bool) {
 func loadMemShift(ins LoadMemShift, in []byte) (uint32, bool) {
 	offset := int(ins.Off)
 
-	if !inBounds(len(in), offset, 0) {
+	// Size of LoadMemShift is always 1 byte
+	if !inBounds(len(in), offset, 1) {
 		return 0, false
 	}
 

bpf/vm_load_test.go

@@ -13,55 +13,61 @@ import (
 )
 
 func TestVMLoadAbsoluteOffsetOutOfBounds(t *testing.T) {
+	pkt := []byte{
+		0xff, 0xff, 0xff, 0xff,
+		0xff, 0xff, 0xff, 0xff,
+		0, 1, 2, 3,
+	}
+
 	vm, done, err := testVM(t, []bpf.Instruction{
 		bpf.LoadAbsolute{
-			Off:  100,
-			Size: 2,
+			Off:  uint32(len(pkt)),
+			Size: 1,
 		},
-		bpf.RetA{},
+		// Out of bounds should return 0, return 1 to tell if execution continued
+		bpf.RetConstant{Val: 1},
 	})
 	if err != nil {
 		t.Fatalf("failed to load BPF program: %v", err)
 	}
 	defer done()
 
-	out, err := vm.Run([]byte{
-		0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff,
-		0, 1, 2, 3,
-	})
+	out, err := vm.Run(pkt)
 	if err != nil {
 		t.Fatalf("unexpected error while running program: %v", err)
 	}
 	if want, got := 0, out; want != got {
-		t.Fatalf("unexpected number of output bytes:\n- want: %d\n-  got: %d",
+		t.Fatalf("unexpected result:\n- want: %d\n-  got: %d",
 			want, got)
 	}
 }
 
 func TestVMLoadAbsoluteOffsetPlusSizeOutOfBounds(t *testing.T) {
+	pkt := []byte{
+		0xff, 0xff, 0xff, 0xff,
+		0xff, 0xff, 0xff, 0xff,
+		0,
+	}
+
 	vm, done, err := testVM(t, []bpf.Instruction{
 		bpf.LoadAbsolute{
-			Off:  8,
+			Off:  uint32(len(pkt) - 1),
 			Size: 2,
 		},
-		bpf.RetA{},
+		// Out of bounds should return 0, return 1 to tell if execution continued
+		bpf.RetConstant{Val: 1},
 	})
 	if err != nil {
 		t.Fatalf("failed to load BPF program: %v", err)
 	}
 	defer done()
 
-	out, err := vm.Run([]byte{
-		0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff,
-		0,
-	})
+	out, err := vm.Run(pkt)
 	if err != nil {
 		t.Fatalf("unexpected error while running program: %v", err)
 	}
 	if want, got := 0, out; want != got {
-		t.Fatalf("unexpected number of output bytes:\n- want: %d\n-  got: %d",
+		t.Fatalf("unexpected result:\n- want: %d\n-  got: %d",
 			want, got)
 	}
 }
@@ -107,54 +113,60 @@ func TestVMLoadConstantOK(t *testing.T) {
 }
 
 func TestVMLoadIndirectOutOfBounds(t *testing.T) {
+	pkt := []byte{
+		0xff, 0xff, 0xff, 0xff,
+		0xff, 0xff, 0xff, 0xff,
+		0,
+	}
+
 	vm, done, err := testVM(t, []bpf.Instruction{
 		bpf.LoadIndirect{
-			Off:  100,
+			Off:  uint32(len(pkt)),
 			Size: 1,
 		},
-		bpf.RetA{},
+		// Out of bounds should return 0, return 1 to tell if execution continued
+		bpf.RetConstant{Val: 1},
 	})
 	if err != nil {
 		t.Fatalf("failed to load BPF program: %v", err)
 	}
 	defer done()
 
-	out, err := vm.Run([]byte{
-		0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff,
-		0,
-	})
+	out, err := vm.Run(pkt)
 	if err != nil {
 		t.Fatalf("unexpected error while running program: %v", err)
 	}
 	if want, got := 0, out; want != got {
-		t.Fatalf("unexpected number of output bytes:\n- want: %d\n-  got: %d",
+		t.Fatalf("unexpected result:\n- want: %d\n-  got: %d",
 			want, got)
 	}
 }
 
 func TestVMLoadMemShiftOutOfBounds(t *testing.T) {
+	pkt := []byte{
+		0xff, 0xff, 0xff, 0xff,
+		0xff, 0xff, 0xff, 0xff,
+		0,
+	}
+
 	vm, done, err := testVM(t, []bpf.Instruction{
 		bpf.LoadMemShift{
-			Off: 100,
+			Off: uint32(len(pkt)),
 		},
-		bpf.RetA{},
+		// Out of bounds should return 0, return 1 to tell if execution continued
+		bpf.RetConstant{Val: 1},
 	})
 	if err != nil {
 		t.Fatalf("failed to load BPF program: %v", err)
 	}
 	defer done()
 
-	out, err := vm.Run([]byte{
-		0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff,
-		0,
-	})
+	out, err := vm.Run(pkt)
 	if err != nil {
 		t.Fatalf("unexpected error while running program: %v", err)
 	}
 	if want, got := 0, out; want != got {
-		t.Fatalf("unexpected number of output bytes:\n- want: %d\n-  got: %d",
+		t.Fatalf("unexpected result:\n- want: %d\n-  got: %d",
 			want, got)
 	}
 }