diff options
author | Ian Gudger <igudger@google.com> | 2018-12-17 17:52:05 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-12-17 17:53:22 -0800 |
commit | 12c7430a01ad2b484987dd8ee24b6f2907e7366d (patch) | |
tree | 832c72f40867827b53bd357708c7cbdd3e6f6842 | |
parent | b62591e6a813ec19a1fd74943584c4fead81f670 (diff) |
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
-rw-r--r-- | pkg/sentry/socket/unix/transport/connectioned.go | 8 | ||||
-rw-r--r-- | pkg/sentry/socket/unix/transport/connectionless.go | 2 | ||||
-rw-r--r-- | pkg/sentry/socket/unix/transport/queue.go | 5 | ||||
-rw-r--r-- | 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<SocketPairKind> GetSocketPairs() { ApplyVec<SocketPairKind>( FilesystemBoundUnixDomainSocketPair, AllBitwiseCombinations( - // FIXME: Add SOCK_DGRAM once blocking is fixed. - List<int>{SOCK_STREAM, SOCK_SEQPACKET}, + List<int>{SOCK_STREAM, SOCK_SEQPACKET, SOCK_DGRAM}, List<int>{0, SOCK_CLOEXEC})), ApplyVec<SocketPairKind>( AbstractBoundUnixDomainSocketPair, AllBitwiseCombinations( - // FIXME: Add SOCK_DGRAM once blocking is fixed. - List<int>{SOCK_STREAM, SOCK_SEQPACKET}, + List<int>{SOCK_STREAM, SOCK_SEQPACKET, SOCK_DGRAM}, List<int>{0, SOCK_CLOEXEC}))); } |