x/net/bpf: bpf VM only supports running programs with big endian #20556
Comments
|
We cannot review patches on GitHub. Please upload your patch to Gerrit by following https://golang.org/doc/contribute.html |
|
I am certainly open to making the endianness configurable (probably via the VM's constructor). Please submit a CL and add me as a reviewer. |
|
Please don't expose a control knob like |
mikioh
changed the title
|
/CC @danderson |
|
@mikioh Hmm, this is interesting. I'm not actually sure where the bug is here. The BPF spec does not mention byte ordering at all. However, the VM supports multi-byte integer load instructions, so it's implicitly assuming something about byte ordering. Empirically, when we use BPF for packet filtering in the kernel, the load operations use big-endian ordering. I know this because The next thing we need to know is: what is the behavior of BPF when it's running within seccomp? The input data with seccomp is a kernel struct that contains syscall information, so it's logical to assume that integers in that payload are native-endian... But does that mean the kernel BPF VM is loading ints differently when running in seccomp?! Once we know what the empirical behavior of seccomp-bpf is inside the kernel, we'll know what to fix: either the userspace VM implementation in this package, or libseccomp-golang, or maybe neither. @mvo5 How well do you know seccomp-bpf? Do you know how the kernel implements integer load ops in that subsystem? I'll try to dig through the kernel source code tonight to find some answers... But we should find these answers before trying to patch anything :) |
|
And by the way, if the bug is actually that the kernel implementation uses different byte ordering modes between packet filtering and seccomp, then my preferred fix would be to break the API of NewVM, and add a So, for example, if you want a packet filter, you would write Thoughts? |
|
I think it's fine to change the signature of NewVM for specifying VM flavors. |
|
@danderson Thanks a lot for looking into this issue! I'm fine with your suggestion to not expose the endianness directly. I'm also not deeply familar with the seccomp-bpf implementation but I did do poke around the code in libseccomp and it has explicit code to convert host integers to the targets endianess and the scmp_bpf_sim.c bpf simulator in the libseccomp code also has code that ensures the data is in the target arches native endianess (c.f. https://github.com/seccomp/libseccomp/blob/master/tools/scmp_bpf_sim.c#L321). I have not looked at the kernel seccomp-bpf code though. |
|
@danderson I had a quick look at the kernel seccomp bpf code and the code states: |
|
I am currently sorting the CLA out, once I have that, I will propose a proper branch. In the meantime here is a diff is someone wants to check if the direction makes sense or give some early feedback: 0001-Add-bpf.VMType-and-modify-VM.NewVM-to-take-it.patch.gz. |
The bpf of seccomp uses native endian. The x/net/bpf always uses the architecture specific endian. This means we can not currently simulate our geneated bpf with the bpf.VM. There is a open bug at golang/go#20556 Given that we now also test the generated bpf against the in-kernel seccomp implementation we can retire the bpf.VM tests (which test exactly the same).
The bpf code of seccomp uses native endian. The x/net/bpf VM always uses the network endian. This means we can not currently simulate our geneated bpf with the bpf.VM. There is a open bug at golang/go#20556 Given that we now also test the generated bpf against the in-kernel seccomp implementation we can retire the bpf.VM tests (which test exactly the same).
Please answer these questions before submitting your issue. Thanks!
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)?GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/egon/devel/go"
GORACE=""
GOROOT="/usr/lib/go-1.7"
GOTOOLDIR="/usr/lib/go-1.7/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build717134135=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
What did you do?
I am using x/net/bpf so that I can simulate a bunch of bpf programs generated by libseccomp-golang in the net/bpf VM to validate a small txt->bpf compiler.
I ran into the problem that the x/net/bpf VM assumes BigEndian byte order. However libseccomp-golang generates code in host byteorder (which happens to be little endian on my machine) so the simulation is not working as expected.
What did you expect to see?
I was hoping the VM would let me control the endianness. Please consider the following diff, it is fully backwards compatible but it will let me run libseccomp generated bpf programs inside the VM.
diff --git a/bpf/vm_instructions.go b/bpf/vm_instructions.go index 516f946..2263c3f 100644 --- a/bpf/vm_instructions.go +++ b/bpf/vm_instructions.go @@ -134,6 +134,10 @@ func inBounds(inLen int, offset int, size int) bool { return offset+size <= inLen } +// VmEndianness controlls what byte order the bpf.VM is using. +// The default is binary.BigEndian +var VmEndianness binary.ByteOrder = binary.BigEndian + func loadCommon(in []byte, offset int, size int) (uint32, bool) { if !inBounds(len(in), offset, size) { return 0, false @@ -143,9 +147,9 @@ func loadCommon(in []byte, offset int, size int) (uint32, bool) { case 1: return uint32(in[offset]), true case 2: - return uint32(binary.BigEndian.Uint16(in[offset : offset+size])), true + return uint32(VmEndianness.Uint16(in[offset : offset+size])), true case 4: - return uint32(binary.BigEndian.Uint32(in[offset : offset+size])), true + return uint32(VmEndianness.Uint32(in[offset : offset+size])), true default: panic(fmt.Sprintf("invalid load size: %d", size)) }The text was updated successfully, but these errors were encountered: