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.

Sign up for GitHub

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

Jump to bottom

fix: add CLOEXEC flag to socket created directly #550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: add CLOEXEC flag to socket created directly #550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions dhcpv4/client4/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/insomniacslk/dhcp/dhcpv4"
"github.com/insomniacslk/dhcp/internal/xsocket"
"golang.org/x/net/ipv4"
"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -77,7 +78,7 @@ func MakeRawUDPPacket(payload []byte, serverAddr, clientAddr net.UDPAddr) ([]byt

// makeRawSocket creates a socket that can be passed to unix.Sendto.
func makeRawSocket(ifname string) (int, error) {
fd, err := unix.Socket(unix.AF_INET, unix.SOCK_RAW, unix.IPPROTO_RAW)
fd, err := xsocket.CloexecSocket(unix.AF_INET, unix.SOCK_RAW, unix.IPPROTO_RAW)
if err != nil {
return fd, err
}
Expand Down Expand Up @@ -123,7 +124,7 @@ func htons(v uint16) uint16 {
}

func makeListeningSocketWithCustomPort(ifname string, port int) (int, error) {
fd, err := unix.Socket(unix.AF_PACKET, unix.SOCK_DGRAM, int(htons(unix.ETH_P_IP)))
fd, err := xsocket.CloexecSocket(unix.AF_PACKET, unix.SOCK_DGRAM, int(htons(unix.ETH_P_IP)))
if err != nil {
return fd, err
}
Expand Down
4 changes: 3 additions & 1 deletion dhcpv4/server4/conn_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !windows
// +build !windows

package server4
Expand All @@ -9,6 +10,7 @@ import (
"os"

"github.com/insomniacslk/dhcp/dhcpv4"
"github.com/insomniacslk/dhcp/internal/xsocket"
"golang.org/x/sys/unix"
)

Expand All @@ -17,7 +19,7 @@ import (
//
// The interface must already be configured.
func NewIPv4UDPConn(iface string, addr *net.UDPAddr) (*net.UDPConn, error) {
fd, err := unix.Socket(unix.AF_INET, unix.SOCK_DGRAM, unix.IPPROTO_UDP)
fd, err := xsocket.CloexecSocket(unix.AF_INET, unix.SOCK_DGRAM|unix.SOCK_CLOEXEC, unix.IPPROTO_UDP)
if err != nil {
return nil, fmt.Errorf("cannot get a UDP socket: %v", err)
}
Expand Down
4 changes: 3 additions & 1 deletion dhcpv6/server6/conn_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !windows
// +build !windows

package server6
Expand All @@ -9,6 +10,7 @@ import (
"os"

"github.com/insomniacslk/dhcp/interfaces"
"github.com/insomniacslk/dhcp/internal/xsocket"
"golang.org/x/sys/unix"
)

Expand All @@ -18,7 +20,7 @@ import (
//
// The interface must already be configured.
func NewIPv6UDPConn(iface string, addr *net.UDPAddr) (*net.UDPConn, error) {
fd, err := unix.Socket(unix.AF_INET6, unix.SOCK_DGRAM, unix.IPPROTO_UDP)
fd, err := xsocket.CloexecSocket(unix.AF_INET6, unix.SOCK_DGRAM, unix.IPPROTO_UDP)
if err != nil {
return nil, fmt.Errorf("cannot get a UDP socket: %v", err)
}
Expand Down
36 changes: 36 additions & 0 deletions internal/xsocket/xsocket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package xsocket

import (
"syscall"

"golang.org/x/sys/unix"
)

// CloexecSocket creates a new socket with the close-on-exec flag set.
//
// If the OS doesn't support the close-on-exec flag, this function will try a workaround.
func CloexecSocket(domain, typ, proto int) (int, error) {
fd, err := unix.Socket(domain, typ|unix.SOCK_CLOEXEC, proto)
if err == nil {
return fd, nil
}

if err == unix.EINVAL || err == unix.EPROTONOSUPPORT {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EINVAL seems awfully wide an error to catch for this I think, especially with having to take the forklock in the fallback case.
Can this be further narrowed per-platform with build tags? Afaict linux and all the go-supported bsds support SOCK_CLOEXEC, so the only actually relevant platform would be darwin, non-illumos solaris (before 11.4 according to net source ), maybe AIX?
It looks like GOOS=darwin doesn't even define unix.SOCK_CLOEXEC so this workaround won't even compile there (would it be necessary? I'm not sure)

I'd be inclined to not support it at all, I don't think any of these platforms are tested at all anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Darwin should support CLOEXEC I believe starting with some version, but I haven't tested it.

There's a similar workaround in Go runtime, and this code is loosely based on https://github.com/mdlayher/socket/blob/9c51a391be6c6bc5b7ce52f328497931c5e97733/conn.go#L254-L303

But I'm happy to drop it and simply do | unix.O_CLOEXEC

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, SOCK_CLOEXEC is missing on Darwin, but other constants are also missing, so not sure how much relevant that is:

# github.com/insomniacslk/dhcp/dhcpv4/nclient4
dhcpv4/nclient4/conn_unix.go:42:59: undefined: unix.ETH_P_IP

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Natolumin do you want me to simplify and just add CLOEXEC directly?

// SOCK_CLOEXEC is not supported, try without it, but avoid racing with fork/exec
syscall.ForkLock.RLock()

fd, err = unix.Socket(domain, typ, proto)
if err != nil {
syscall.ForkLock.RUnlock()
return -1, err
}

unix.CloseOnExec(fd)

syscall.ForkLock.RUnlock()

return fd, nil
}

return fd, err
}
Loading