summaryrefslogtreecommitdiffhomepage
path: root/pkg
diff options
context:
space:
mode:
authorNicolas Lacasse <nlacasse@google.com>2021-02-19 17:36:44 -0800
committergVisor bot <gvisor-bot@google.com>2021-02-19 17:38:51 -0800
commit93fc09248a2fa8b840d8ce47800414980d74bdb0 (patch)
tree6006fc6b336315a4ae272782b8afb5c1130a9b2b /pkg
parent7544eeb242d0aba2da054a1663e043feaedb9618 (diff)
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
Diffstat (limited to 'pkg')
-rw-r--r--pkg/sentry/kernel/epoll/epoll.go6
-rw-r--r--pkg/sentry/socket/unix/transport/unix.go47
-rw-r--r--pkg/sentry/vfs/vfs.go4
3 files changed, 37 insertions, 20 deletions
diff --git a/pkg/sentry/kernel/epoll/epoll.go b/pkg/sentry/kernel/epoll/epoll.go
index 61aeca044..407b6e917 100644
--- a/pkg/sentry/kernel/epoll/epoll.go
+++ b/pkg/sentry/kernel/epoll/epoll.go
@@ -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 (
diff --git a/pkg/sentry/socket/unix/transport/unix.go b/pkg/sentry/socket/unix/transport/unix.go
index ceada54a8..359a5995b 100644
--- a/pkg/sentry/socket/unix/transport/unix.go
+++ b/pkg/sentry/socket/unix/transport/unix.go
@@ -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
diff --git a/pkg/sentry/vfs/vfs.go b/pkg/sentry/vfs/vfs.go
index b0e13cdab..00f1847d8 100644
--- a/pkg/sentry/vfs/vfs.go
+++ b/pkg/sentry/vfs/vfs.go
@@ -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