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

syscall: panic in syscall.ParseNetlinkMessage when attempting to copy past slice end #18714

Closed
elevran opened this issue Jan 19, 2017 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@elevran
Copy link

elevran commented Jan 19, 2017

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/elevran/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build112723414=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

Use netfilter subsystem to deliver traffic to user space using https://github.com/elevran/nfq.
Running instructions to reproduce are in README. The failure is not deterministic, and depends on actual messages sent by kernel. An example failure is documented in subgraph/go-nfnetlink#3, which fixes an issue with the go-nfnetlink library.

What did you expect to see?

All valid packets of the connection are processed by the usersapce program

What did you see instead?

Panic in syscall.ParseNetlinkMessage

Note

This is a duplicate of #16681, which was closed and planned for 1.8.
I believe the fix in 6fd8c00 can be improved. The current suggestion logs an error and drops packets, which I think is too strict.
To the best of my understanding there is no hard and clear requirement that netlink messages are always aligned in size, only that they should start at an aligned address. This leaves room for a case where the last message has a size which is not a multiple of the required alignment, causing the parsing call to attempt accessing past the slice end.

@elevran elevran changed the title Panic in syscall.ParseNetlinkMessage syscall: panic in syscall.ParseNetlinkMessage when attempting to copy past slice end Jan 19, 2017
@bradfitz
Copy link
Contributor

So did you try Go 1.8?

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 19, 2017
@elevran
Copy link
Author

elevran commented Jan 20, 2017

@bradfitz
I have.
An EINVAL is returned from the parsing which skips packet processing. Only two packets are processed before the userspace program receives (indirectly) an invalid argument error from ParseNetlinkMessage using the go 1.8rc2 code base.

The code is in the subgraph branch, since master has a fix that bypasses the issue in the go-nfnetlink package itself.

Sample run demonstrating issue with 1.8rc2 code base below. I'd be happy to submit a patch that accepts misaligned netlink message lengths, if that's of interest

build and run test program

$ go version
go version go1.8rc2 linux/amd64
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/elevran/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build882606754=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
$ git checkout subgraph
$ go build .
$ sudo ./nfq

Packet: PACKET: 60 bytes
- Layer 1 (20 bytes) = IPv4	{Contents=[..20..] Payload=[..40..] Version=4 IHL=5 TOS=0 Length=60 Id=11896 Flags=DF FragOffset=0 TTL=64 Protocol=TCP Checksum=16952 SrcIP=10.0.2.15 DstIP=172.217.17.36 Options=[] Padding=[]}
- Layer 2 (40 bytes) = TCP	{Contents=[..40..] Payload=[] SrcPort=36738 DstPort=80(http) Seq=2493731064 Ack=0 DataOffset=10 FIN=false SYN=true RST=false PSH=false ACK=false URG=false ECE=false CWR=false NS=false Window=29200 Checksum=6124 Urgent=0 Options=[..5..] Padding=[]}

Packet: PACKET: 40 bytes
- Layer 1 (20 bytes) = IPv4	{Contents=[..20..] Payload=[..20..] Version=4 IHL=5 TOS=0 Length=40 Id=11897 Flags=DF FragOffset=0 TTL=64 Protocol=TCP Checksum=16971 SrcIP=10.0.2.15 DstIP=172.217.17.36 Options=[] Padding=[]}
- Layer 2 (20 bytes) = TCP	{Contents=[..20..] Payload=[] SrcPort=36738 DstPort=80(http) Seq=2493731065 Ack=354624002 DataOffset=5 FIN=false SYN=false RST=false PSH=false ACK=true URG=false ECE=false CWR=false NS=false Window=29200 Checksum=50979 Urgent=0 Options=[] Padding=[]}

Error from socket receive: invalid argument - closed channel
Exiting after 2 packets

$ wget www.google.com

--2017-01-20 10:47:41--  http://www.google.com/
Resolving www.google.com (www.google.com)... 172.217.17.36, 2a00:1450:400e:805::2004
Connecting to www.google.com (www.google.com)|172.217.17.36|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: http://www.google.co.il/?gfe_rd=cr&ei=rc6BWM3jDKGk8wf927KADA [following]
--2017-01-20 10:47:42--  http://www.google.co.il/?gfe_rd=cr&ei=rc6BWM3jDKGk8wf927KADA
Resolving www.google.co.il (www.google.co.il)... 172.217.17.67, 2a00:1450:400e:804::2003
Connecting to www.google.co.il (www.google.co.il)|172.217.17.67|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: unspecified [text/html]
Saving to: ‘index.html.1’

index.html.1            [ <=>                ]  10.94K  --.-KB/s    in 0.001s

2017-01-20 10:47:42 (8.61 MB/s) - ‘index.html.1’ saved [11199]

@bradfitz
Copy link
Contributor

Sure, send a change via Gerrit? See https://golang.org/doc/contribute.html

@elevran
Copy link
Author

elevran commented Jan 21, 2017

@gopherbot
Copy link

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

@bradfitz bradfitz added this to the Go1.9 milestone Jan 25, 2017
@bradfitz bradfitz removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 25, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jul 14, 2017
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Unplanned Dec 13, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Oct 3, 2018

I'm closing this as WontFix. My comment on the CL:

It sounds like there isn't consensus on what to do here, so the default is to not accept this, especially since it's in the syscall package which is frozen. Development should happen in x/sys/unix or x/net.

@bradfitz bradfitz closed this as completed Oct 3, 2018
@golang golang locked and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants