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

tcpip/link/fdbased: socket operation on non-socket #7125

Closed
xjasonlyu opened this issue Feb 1, 2022 · 2 comments
Closed

tcpip/link/fdbased: socket operation on non-socket #7125

xjasonlyu opened this issue Feb 1, 2022 · 2 comments
Assignees
Labels
type: bug Something isn't working

Comments

@xjasonlyu
Copy link

Description

Since commit e511fc9 removed the WritePacket method from tcpip/link/fdbased package, this error may occur under some certain circumstances.

e.g. When we use tcpip/link/tun to open a TUN device:

fd, err := tun.Open(t.name)
if err != nil {
	return nil, fmt.Errorf("create tun: %w", err)
}

Then pass the TUN fd as a fdbased.Option and add the endpoint to stack:

ep, err := fdbased.New(&fdbased.Options{
	FDs: []int{fd},
	...
})

The WritePackets method would be called to write back packets:

func (e *endpoint) WritePackets(_ stack.RouteInfo, pkts stack.PacketBufferList, _ tcpip.NetworkProtocolNumber) (int, tcpip.Error) {

Then the sendBatch:

n, err := e.sendBatch(batchFD, batch)

However, in the the sendBatch, the else would be hit and call the rawfile.NonBlockingSendMMsg(batchFD, mmsgHdrs):

if len(mmsgHdrs) == 0 {
// We can't fit batch[0] into a mmsghdr while staying under
// e.maxSyscallHeaderBytes. Use WritePacket, which will avoid the
// mmsghdr (by using writev) and re-buffer iovecs more aggressively
// if necessary (by using e.writevMaxIovs instead of
// rawfile.MaxIovs).
pkt := batch[0]
if err := e.writePacket(pkt.EgressRoute, pkt.NetworkProtocolNumber, pkt); err != nil {
return packets, err
}
packets++
} else {
for len(mmsgHdrs) > 0 {
sent, err := rawfile.NonBlockingSendMMsg(batchFD, mmsgHdrs)
if err != nil {
return packets, err
}
packets += sent
mmsgHdrs = mmsgHdrs[sent:]
}
}

So the issue happens here, the SENDMMSG syscall is used on a non-socket fd (TUN fd instead) and cause the socket operation on non-socket error:

func NonBlockingSendMMsg(fd int, msgHdrs []MMsgHdr) (int, tcpip.Error) {
n, _, e := unix.RawSyscall6(unix.SYS_SENDMMSG, uintptr(fd), uintptr(unsafe.Pointer(&msgHdrs[0])), uintptr(len(msgHdrs)), unix.MSG_DONTWAIT, 0, 0)
if e != 0 {
return 0, TranslateErrno(e)
}
return int(n), nil
}

Steps to reproduce

Mentioned above.

runsc version

null

docker version (if using docker)

null

uname

null

kubectl (if using Kubernetes)

null

repo state (if built from source)

null

runsc debug logs (if available)

null
@xjasonlyu xjasonlyu added the type: bug Something isn't working label Feb 1, 2022
@hbhasker hbhasker self-assigned this Feb 1, 2022
@hbhasker
Copy link
Contributor

hbhasker commented Feb 1, 2022

Thanks for the detailed report. I will take a look.

@hbhasker
Copy link
Contributor

hbhasker commented Feb 1, 2022

We just need to store that the underlying FD is not a socket and degrade WritePackets to writePacket always.

We do the socket check fdbased.New() but its not plumbed through to the write path. We should do that.

copybara-service bot pushed a commit that referenced this issue Feb 5, 2022
TAP/TUN fd's are not sockets and using the WritePackets calls results
in errors as it always defaults to using SendMMsg which is not supported
for tap/tun device fds.

This CL changes WritePackets to gracefully degrade to using writev instead
of sendmmsg if the underlying fd is not a socket.

Fixes #7125

RELNOTES[servinf]: n/a. Does not impact serverless as tun/tap is not used by sentry.

PiperOrigin-RevId: 425992492
xjasonlyu added a commit to xjasonlyu/tun2socks that referenced this issue Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants