From 12c7430a01ad2b484987dd8ee24b6f2907e7366d Mon Sep 17 00:00:00 2001 From: Ian Gudger Date: Mon, 17 Dec 2018 17:52:05 -0800 Subject: Fix recv blocking for connectionless Unix sockets. Connectionless Unix sockets (DGRAM Unix sockets created with the socket system call) inherently only have a read queue. They do not establish bidirectional connections, instead, the connect system call only sets a default send location. Writes give the data to the other endpoint which has its own read queue. To simplify the code, connectionless Unix sockets still get read and write queues, but the write queue is a dummy and never waited on. The read queue is the connectionless endpoint's queue. This change fixes a bug where the dummy queue was incorrectly set as the read queue and the endpoint's queue was incorrectly set as the write queue. This meant that read notifications went to the dummy queue and were black holed. PiperOrigin-RevId: 225921042 Change-Id: I8d9059def787a2c3c305185b92d05093fbd2be2a --- pkg/sentry/socket/unix/transport/connectioned.go | 8 ++++---- pkg/sentry/socket/unix/transport/connectionless.go | 2 +- pkg/sentry/socket/unix/transport/queue.go | 5 ----- test/syscalls/linux/socket_unix_blocking_local.cc | 6 ++---- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/pkg/sentry/socket/unix/transport/connectioned.go b/pkg/sentry/socket/unix/transport/connectioned.go index 7cfbbfe8a..62641bb34 100644 --- a/pkg/sentry/socket/unix/transport/connectioned.go +++ b/pkg/sentry/socket/unix/transport/connectioned.go @@ -135,8 +135,8 @@ func NewPair(stype SockType, uid UniqueIDProvider) (Endpoint, Endpoint) { stype: stype, } - q1 := newQueue(a.Queue, b.Queue, initialLimit) - q2 := newQueue(b.Queue, a.Queue, initialLimit) + q1 := &queue{ReaderQueue: a.Queue, WriterQueue: b.Queue, limit: initialLimit} + q2 := &queue{ReaderQueue: b.Queue, WriterQueue: a.Queue, limit: initialLimit} if stype == SockStream { a.receiver = &streamQueueReceiver{queueReceiver: queueReceiver{q1}} @@ -286,13 +286,13 @@ func (e *connectionedEndpoint) BidirectionalConnect(ce ConnectingEndpoint, retur stype: e.stype, } - readQueue := newQueue(ce.WaiterQueue(), ne.Queue, initialLimit) + readQueue := &queue{ReaderQueue: ce.WaiterQueue(), WriterQueue: ne.Queue, limit: initialLimit} ne.connected = &connectedEndpoint{ endpoint: ce, writeQueue: readQueue, } - writeQueue := newQueue(ne.Queue, ce.WaiterQueue(), initialLimit) + writeQueue := &queue{ReaderQueue: ne.Queue, WriterQueue: ce.WaiterQueue(), limit: initialLimit} if e.stype == SockStream { ne.receiver = &streamQueueReceiver{queueReceiver: queueReceiver{readQueue: writeQueue}} } else { diff --git a/pkg/sentry/socket/unix/transport/connectionless.go b/pkg/sentry/socket/unix/transport/connectionless.go index f432a9717..728863f3f 100644 --- a/pkg/sentry/socket/unix/transport/connectionless.go +++ b/pkg/sentry/socket/unix/transport/connectionless.go @@ -34,7 +34,7 @@ type connectionlessEndpoint struct { // NewConnectionless creates a new unbound dgram endpoint. func NewConnectionless() Endpoint { ep := &connectionlessEndpoint{baseEndpoint{Queue: &waiter.Queue{}}} - ep.receiver = &queueReceiver{readQueue: newQueue(&waiter.Queue{}, ep.Queue, initialLimit)} + ep.receiver = &queueReceiver{readQueue: &queue{ReaderQueue: ep.Queue, WriterQueue: &waiter.Queue{}, limit: initialLimit}} return ep } diff --git a/pkg/sentry/socket/unix/transport/queue.go b/pkg/sentry/socket/unix/transport/queue.go index 05d1bdeef..45a58c600 100644 --- a/pkg/sentry/socket/unix/transport/queue.go +++ b/pkg/sentry/socket/unix/transport/queue.go @@ -38,11 +38,6 @@ type queue struct { dataList messageList } -// newQueue allocates and initializes a new queue. -func newQueue(ReaderQueue *waiter.Queue, WriterQueue *waiter.Queue, limit int64) *queue { - return &queue{ReaderQueue: ReaderQueue, WriterQueue: WriterQueue, limit: limit} -} - // Close closes q for reading and writing. It is immediately not writable and // will become unreadable when no more data is pending. // diff --git a/test/syscalls/linux/socket_unix_blocking_local.cc b/test/syscalls/linux/socket_unix_blocking_local.cc index f79e04b33..3c2105cc7 100644 --- a/test/syscalls/linux/socket_unix_blocking_local.cc +++ b/test/syscalls/linux/socket_unix_blocking_local.cc @@ -33,14 +33,12 @@ std::vector GetSocketPairs() { ApplyVec( FilesystemBoundUnixDomainSocketPair, AllBitwiseCombinations( - // FIXME: Add SOCK_DGRAM once blocking is fixed. - List{SOCK_STREAM, SOCK_SEQPACKET}, + List{SOCK_STREAM, SOCK_SEQPACKET, SOCK_DGRAM}, List{0, SOCK_CLOEXEC})), ApplyVec( AbstractBoundUnixDomainSocketPair, AllBitwiseCombinations( - // FIXME: Add SOCK_DGRAM once blocking is fixed. - List{SOCK_STREAM, SOCK_SEQPACKET}, + List{SOCK_STREAM, SOCK_SEQPACKET, SOCK_DGRAM}, List{0, SOCK_CLOEXEC}))); } -- cgit v1.2.3