Don't hold baseEndpoint.mu while calling EventUpdate().

This removes a three-lock deadlock between fdnotifier.notifier.mu,
epoll.EventPoll.listsMu, and baseEndpoint.mu.

A lock order comment was added to epoll/epoll.go.

Also fix unsafe access of baseEndpoint.connected/receiver.

PiperOrigin-RevId: 358515191
This commit is contained in:
Nicolas Lacasse
2021-02-19 17:38:51 -08:00
committed by gVisor bot
parent 7544eeb242
commit 93fc09248a
3 changed files with 37 additions and 20 deletions
+6
View File
@@ -14,6 +14,12 @@
// Package epoll provides an implementation of Linux's IO event notification
// facility. See epoll(7) for more details.
//
// Lock order:
// EventPoll.mu
// fdnotifier.notifier.mu
// EventPoll.listsMu
// unix.baseEndpoint.Mutex
package epoll
import (
+28 -19
View File
@@ -742,8 +742,8 @@ func (e *connectedEndpoint) SetSendBufferSize(v int64) (newSz int64) {
return v
}
// baseEndpoint is an embeddable unix endpoint base used in both the connected and connectionless
// unix domain socket Endpoint implementations.
// baseEndpoint is an embeddable unix endpoint base used in both the connected
// and connectionless unix domain socket Endpoint implementations.
//
// Not to be used on its own.
//
@@ -753,6 +753,9 @@ type baseEndpoint struct {
tcpip.DefaultSocketOptionsHandler
// Mutex protects the below fields.
//
// See the lock ordering comment in package kernel/epoll regarding when
// this lock can safely be held.
sync.Mutex `state:"nosave"`
// receiver allows Messages to be received.
@@ -774,20 +777,22 @@ type baseEndpoint struct {
func (e *baseEndpoint) EventRegister(we *waiter.Entry, mask waiter.EventMask) {
e.Queue.EventRegister(we, mask)
e.Lock()
if e.connected != nil {
e.connected.EventUpdate()
}
c := e.connected
e.Unlock()
if c != nil {
c.EventUpdate()
}
}
// EventUnregister implements waiter.Waitable.EventUnregister.
func (e *baseEndpoint) EventUnregister(we *waiter.Entry) {
e.Queue.EventUnregister(we)
e.Lock()
if e.connected != nil {
e.connected.EventUpdate()
}
c := e.connected
e.Unlock()
if c != nil {
c.EventUpdate()
}
}
// Passcred implements Credentialer.Passcred.
@@ -942,22 +947,26 @@ func (e *baseEndpoint) Shutdown(flags tcpip.ShutdownFlags) *syserr.Error {
return syserr.ErrNotConnected
}
if flags&tcpip.ShutdownRead != 0 {
e.receiver.CloseRecv()
var (
r = e.receiver
c = e.connected
shutdownRead = flags&tcpip.ShutdownRead != 0
shutdownWrite = flags&tcpip.ShutdownWrite != 0
)
if shutdownRead {
r.CloseRecv()
}
if flags&tcpip.ShutdownWrite != 0 {
e.connected.CloseSend()
if shutdownWrite {
c.CloseSend()
}
e.Unlock()
if flags&tcpip.ShutdownRead != 0 {
e.receiver.CloseNotify()
// Don't hold e.Mutex while calling CloseNotify.
if shutdownRead {
r.CloseNotify()
}
if flags&tcpip.ShutdownWrite != 0 {
e.connected.CloseNotify()
if shutdownWrite {
c.CloseNotify()
}
return nil
+3 -1
View File
@@ -23,7 +23,9 @@
// Dentry.mu
// Locks acquired by FilesystemImpls between Prepare{Delete,Rename}Dentry and Commit{Delete,Rename*}Dentry
// VirtualFilesystem.filesystemsMu
// EpollInstance.mu
// fdnotifier.notifier.mu
// EpollInstance.mu
// Locks acquired by FileDescriptionImpl.Readiness
// Inotify.mu
// Watches.mu
// Inotify.evMu