From d44862f6725c585792e4181b74c74bc82a9d5886 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 12 May 2022 18:09:36 -0700 Subject: [PATCH] sentry/socket: use lockdep mutexes PiperOrigin-RevId: 448382489 --- pkg/sentry/socket/unix/transport/BUILD | 37 +++++++++++++++++++ .../socket/unix/transport/connectioned.go | 26 ++++++++----- pkg/sentry/socket/unix/transport/queue.go | 3 +- pkg/sentry/socket/unix/transport/unix.go | 5 +-- 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/pkg/sentry/socket/unix/transport/BUILD b/pkg/sentry/socket/unix/transport/BUILD index 182567a1b..df0a7f163 100644 --- a/pkg/sentry/socket/unix/transport/BUILD +++ b/pkg/sentry/socket/unix/transport/BUILD @@ -3,6 +3,39 @@ load("//tools/go_generics:defs.bzl", "go_template_instance") package(licenses = ["notice"]) +go_template_instance( + name = "queue_mutex", + out = "queue_mutex.go", + package = "transport", + prefix = "queue", + substrs = { + "genericMark": "unixQueue", + }, + template = "//pkg/sync/locking:generic_mutex", +) + +go_template_instance( + name = "stream_queue_receiver_mutex", + out = "stream_queue_receiver_mutex.go", + package = "transport", + prefix = "streamQueueReceiver", + substrs = { + "genericMark": "streamQueueReceiver", + }, + template = "//pkg/sync/locking:generic_mutex", +) + +go_template_instance( + name = "endpoint_mutex", + out = "endpoint_mutex.go", + package = "transport", + prefix = "endpoint", + substrs = { + "genericMark": "unixEndpoint", + }, + template = "//pkg/sync/locking:generic_mutex", +) + go_template_instance( name = "transport_message_list", out = "transport_message_list.go", @@ -44,13 +77,16 @@ go_library( "connectioned_state.go", "connectionless.go", "connectionless_state.go", + "endpoint_mutex.go", "host.go", "host_connected_endpoint_refs.go", "host_iovec.go", "host_unsafe.go", "queue.go", + "queue_mutex.go", "queue_refs.go", "save_restore.go", + "stream_queue_receiver_mutex.go", "transport_message_list.go", "unix.go", ], @@ -70,6 +106,7 @@ go_library( "//pkg/sentry/inet", "//pkg/sentry/uniqueid", "//pkg/sync", + "//pkg/sync/locking", "//pkg/syserr", "//pkg/tcpip", "//pkg/tcpip/buffer", diff --git a/pkg/sentry/socket/unix/transport/connectioned.go b/pkg/sentry/socket/unix/transport/connectioned.go index 3d86c7726..108f6bcd9 100644 --- a/pkg/sentry/socket/unix/transport/connectioned.go +++ b/pkg/sentry/socket/unix/transport/connectioned.go @@ -23,12 +23,18 @@ import ( "gvisor.dev/gvisor/pkg/fdnotifier" "gvisor.dev/gvisor/pkg/lisafs" "gvisor.dev/gvisor/pkg/sentry/uniqueid" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/syserr" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/waiter" ) +type locker interface { + Lock() + Unlock() + NestedLock() + NestedUnlock() +} + // A ConnectingEndpoint is a connectioned unix endpoint that is attempting to // establish a bidirectional connection with a BoundEndpoint. type ConnectingEndpoint interface { @@ -51,7 +57,7 @@ type ConnectingEndpoint interface { // Locker protects the following methods. While locked, only the holder of // the lock can change the return value of the protected methods. - sync.Locker + locker // Connected returns true iff the ConnectingEndpoint is in the connected // state. ConnectingEndpoints can only be connected to a single endpoint, @@ -292,27 +298,27 @@ func (e *connectionedEndpoint) BidirectionalConnect(ctx context.Context, ce Conn // Do a dance to safely acquire locks on both endpoints. if e.id < ce.ID() { e.Lock() - ce.Lock() + ce.NestedLock() } else { ce.Lock() - e.Lock() + e.NestedLock() } // Check connecting state. if ce.Connected() { - e.Unlock() + e.NestedUnlock() ce.Unlock() return syserr.ErrAlreadyConnected } if ce.ListeningLocked() { - e.Unlock() + e.NestedUnlock() ce.Unlock() return syserr.ErrInvalidEndpointState } // Check bound state. if !e.ListeningLocked() { - e.Unlock() + e.NestedUnlock() ce.Unlock() return syserr.ErrConnectionRefused } @@ -363,7 +369,7 @@ func (e *connectionedEndpoint) BidirectionalConnect(ctx context.Context, ce Conn } // Notify can deadlock if we are holding these locks. - e.Unlock() + e.NestedUnlock() ce.Unlock() // Notify on both ends. @@ -373,9 +379,9 @@ func (e *connectionedEndpoint) BidirectionalConnect(ctx context.Context, ce Conn return nil default: // Busy; return EAGAIN per spec. - ne.Close(ctx) - e.Unlock() + e.NestedUnlock() ce.Unlock() + ne.Close(ctx) return syserr.ErrTryAgain } } diff --git a/pkg/sentry/socket/unix/transport/queue.go b/pkg/sentry/socket/unix/transport/queue.go index 188ad3bd9..3482aef8a 100644 --- a/pkg/sentry/socket/unix/transport/queue.go +++ b/pkg/sentry/socket/unix/transport/queue.go @@ -16,7 +16,6 @@ package transport import ( "gvisor.dev/gvisor/pkg/context" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/syserr" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/buffer" @@ -32,7 +31,7 @@ type queue struct { ReaderQueue *waiter.Queue WriterQueue *waiter.Queue - mu sync.Mutex `state:"nosave"` + mu queueMutex `state:"nosave"` closed bool unread bool used int64 diff --git a/pkg/sentry/socket/unix/transport/unix.go b/pkg/sentry/socket/unix/transport/unix.go index 0840731b7..912605335 100644 --- a/pkg/sentry/socket/unix/transport/unix.go +++ b/pkg/sentry/socket/unix/transport/unix.go @@ -20,7 +20,6 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/lisafs" "gvisor.dev/gvisor/pkg/log" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/syserr" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/buffer" @@ -423,7 +422,7 @@ func (q *queueReceiver) Release(ctx context.Context) { type streamQueueReceiver struct { queueReceiver - mu sync.Mutex `state:"nosave"` + mu streamQueueReceiverMutex `state:"nosave"` buffer []byte control ControlMessages addr tcpip.FullAddress @@ -765,7 +764,7 @@ type baseEndpoint struct { // // See the lock ordering comment in package kernel/epoll regarding when // this lock can safely be held. - sync.Mutex `state:"nosave"` + endpointMutex `state:"nosave"` // receiver allows Messages to be received. receiver Receiver