Skip to content

Commit

Permalink
net/sockerpair_unix: avoid closing unrelated fds
Browse files Browse the repository at this point in the history
We were calling the close() syscall multiple time
with the same value, leading to random issues like
closing containerd stream port.

In newLaunchedPlugin() we have:
sockets, _ := net.NewSocketPair()
defer sockets.Close()
conn, _ := sockets.LocalConn()
peerFile := sockets.PeerFile()
defer func() {
  peerFile.Close()
  if retErr != nil {
    conn.Close()
  }
}()
cmd.Start()

so we were doing:
close(local) (in LocalConn())
cmd.Start()
close(peer) (peerFile.Close())
close(local) (sockets.Close())
close(peer) (sockets.Close())

If the NRI plugin that we launch with cmd.Start() is not cached or
the system is a bit busy, cmd.Start() gives a large enough window
for another goroutine to open a file that will get the same fd number
as local was, thus being closed by accident.

Fix the situation by storing os.Files instead of ints, so that closing
multiple times just returns an error.
  • Loading branch information
champtar committed Jan 29, 2024
1 parent 6f5a4d2 commit 19bd827
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions pkg/net/socketpair_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,31 @@ const (
)

// SocketPair contains the file descriptors of a connected pair of sockets.
type SocketPair [2]int
type SocketPair [2]*os.File

// NewSocketPair returns a connected pair of sockets.
func NewSocketPair() (SocketPair, error) {
fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_STREAM, 0)
if err != nil {
return [2]int{-1, -1}, fmt.Errorf("failed to create socketpair: %w", err)
return SocketPair{nil, nil}, fmt.Errorf("failed to create socketpair: %w", err)
}

return fds, nil
filename := fmt.Sprintf("socketpair-#%d:%d", fds[local], fds[peer])

return SocketPair{
os.NewFile(uintptr(fds[local]), filename+"[0]"),
os.NewFile(uintptr(fds[peer]), filename+"[1]"),
}, nil
}

// LocalFile returns the socketpair fd for local usage as an *os.File.
func (fds SocketPair) LocalFile() *os.File {
return os.NewFile(uintptr(fds[local]), fds.fileName()+"[0]")
return fds[local]
}

// PeerFile returns the socketpair fd for peer usage as an *os.File.
func (fds SocketPair) PeerFile() *os.File {
return os.NewFile(uintptr(fds[peer]), fds.fileName()+"[1]")
return fds[peer]
}

// LocalConn returns a net.Conn for the local end of the socketpair.
Expand All @@ -60,7 +65,7 @@ func (fds SocketPair) LocalConn() (net.Conn, error) {
defer file.Close()
conn, err := net.FileConn(file)
if err != nil {
return nil, fmt.Errorf("failed to create net.Conn for %s[0]: %w", fds.fileName(), err)
return nil, fmt.Errorf("failed to create net.Conn for %s: %w", file.Name(), err)
}
return conn, nil
}
Expand All @@ -71,7 +76,7 @@ func (fds SocketPair) PeerConn() (net.Conn, error) {
defer file.Close()
conn, err := net.FileConn(file)
if err != nil {
return nil, fmt.Errorf("failed to create net.Conn for %s[1]: %w", fds.fileName(), err)
return nil, fmt.Errorf("failed to create net.Conn for %s: %w", file.Name(), err)
}
return conn, nil
}
Expand All @@ -84,14 +89,10 @@ func (fds SocketPair) Close() {

// LocalClose closes the local end of the socketpair.
func (fds SocketPair) LocalClose() {
syscall.Close(fds[local])
fds[local].Close()
}

// PeerClose closes the peer end of the socketpair.
func (fds SocketPair) PeerClose() {
syscall.Close(fds[peer])
}

func (fds SocketPair) fileName() string {
return fmt.Sprintf("socketpair-#%d:%d[0]", fds[local], fds[peer])
fds[peer].Close()
}

0 comments on commit 19bd827

Please sign in to comment.