Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/net/bpf: proper disassemble "fake" jump conditions #18470

Closed
breml opened this issue Dec 29, 2016 · 2 comments
Closed

x/net/bpf: proper disassemble "fake" jump conditions #18470

breml opened this issue Dec 29, 2016 · 2 comments

Comments

@breml
Copy link
Contributor

@breml breml commented Dec 29, 2016

What version of Go are you using (go version)?

go version go1.7.4 linux/amd64

What operating system and processor architecture are you using (go env)?

linux, amd64

What did you do?

Disassemble bpf conditional jump instructions: jneq (jne), jlt, jle.

package main

import (
	"log"
	"reflect"
	"golang.org/x/net/bpf"
)

var allInstructions = []bpf.Instruction{
	bpf.JumpIf{Cond: bpf.JumpNotEqual, Val: 42, SkipTrue: 8},
	bpf.JumpIf{Cond: bpf.JumpLessThan, Val: 42, SkipTrue: 7},
	bpf.JumpIf{Cond: bpf.JumpLessOrEqual, Val: 42, SkipTrue: 6},
}

func main() {
	for _, instr := range allInstructions {
		if jumpIfInstr, ok := instr.(bpf.JumpIf); ok {
			gotAsm, err := jumpIfInstr.Assemble()
			if err != nil {
				log.Fatalf("assembly of '%#v' failed: %s", jumpIfInstr, err)
			}
			got := gotAsm.Disassemble()
			if !reflect.DeepEqual(jumpIfInstr, got) {
				log.Printf("program mutated by disassembly, expected: %#v, got: %#v", jumpIfInstr, got)
			}
		}
	}

}

What did you expect to see?

bpf.JumpIf with condition bpf.JumpNotEqual, bpf.JumpLessThan and bpf.JumpLessOrEqual should disassemble to the same types again.

What did you see instead?

Conditions are disassebled to bpf.JumpEqual, bpf.JumpGreaterOrEqual and bpf.JumpGreaterThan.

Similar issue as: #18469

With both issues (this one and #18469) resolved, the test in https://github.com/golang/net/blob/master/bpf/instructions_test.go#L145-L184 could be simplified without the special handling for "fake" jump conditions.

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 29, 2016

CL https://golang.org/cl/34772 mentions this issue.

@rsc rsc added this to the Unreleased milestone Jan 4, 2017
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 7, 2017

CL https://golang.org/cl/34951 mentions this issue.

breml pushed a commit to breml/bpfutils that referenced this issue Jan 7, 2017
Adopt to the merged changes in golang.org/x/net/bpf

golang/go#18470 and golang/go#18470
gopherbot pushed a commit to golang/net that referenced this issue Jan 8, 2017
The "fake" jump conditions as well as the LoadExtension instructions
are now disassembled correctly. Therefore the workaround to reassemble
the disassembly is no longer necessary.

This simplification was annonced already in golang/go#18470.

Result of `go test -cover .` stays the same with this simplification.

$ go test -cover golang.org/x/net/bpf
ok  	golang.org/x/net/bpf	0.495s	coverage: 92.3% of statements

Change-Id: I3f9eb46148287c76059437b773b80c4c99eb5b53
Reviewed-on: https://go-review.googlesource.com/34951
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>
@golang golang locked and limited conversation to collaborators Jan 7, 2018
NET12115 added a commit to NET12115/Golang-C-NET that referenced this issue Feb 28, 2022
JumpNotEqual, JumpLessThan and JumpLessOrEqual are "fake" jump
conditions that don't appear in the machine code. Nevertheless
these instructions (jneq/jne, jlt, jle) are listed in the
specification and therefore they should be reconstructed from the
machine code.

Fixes golang/go#18470

Specification: https://www.kernel.org/doc/Documentation/networking/filter.txt

Change-Id: I9116b99056e379d89e71adc90516c6747d388e5d
Reviewed-on: https://go-review.googlesource.com/34772
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants