From 0bdcee38bdfa5c4585d28a0edd0c46e170cdc9b5 Mon Sep 17 00:00:00 2001 From: Dean Deng Date: Mon, 26 Oct 2020 14:13:55 -0700 Subject: [PATCH] Fix SCM Rights S/R reference leak. Control messages collected when peeking into a socket were being leaked. PiperOrigin-RevId: 339114961 --- pkg/sentry/socket/unix/transport/unix.go | 12 ++++++++++-- test/syscalls/linux/socket_unix_cmsg.cc | 4 +--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/sentry/socket/unix/transport/unix.go b/pkg/sentry/socket/unix/transport/unix.go index a7d1068a6..b648273a4 100644 --- a/pkg/sentry/socket/unix/transport/unix.go +++ b/pkg/sentry/socket/unix/transport/unix.go @@ -32,6 +32,8 @@ import ( const initialLimit = 16 * 1024 // A RightsControlMessage is a control message containing FDs. +// +// +stateify savable type RightsControlMessage interface { // Clone returns a copy of the RightsControlMessage. Clone() RightsControlMessage @@ -336,7 +338,7 @@ type Receiver interface { RecvMaxQueueSize() int64 // Release releases any resources owned by the Receiver. It should be - // called before droping all references to a Receiver. + // called before dropping all references to a Receiver. Release(ctx context.Context) } @@ -572,6 +574,12 @@ func (q *streamQueueReceiver) Recv(ctx context.Context, data [][]byte, wantCreds return copied, copied, c, cmTruncated, q.addr, notify, nil } +// Release implements Receiver.Release. +func (q *streamQueueReceiver) Release(ctx context.Context) { + q.queueReceiver.Release(ctx) + q.control.Release(ctx) +} + // A ConnectedEndpoint is an Endpoint that can be used to send Messages. type ConnectedEndpoint interface { // Passcred implements Endpoint.Passcred. @@ -619,7 +627,7 @@ type ConnectedEndpoint interface { SendMaxQueueSize() int64 // Release releases any resources owned by the ConnectedEndpoint. It should - // be called before droping all references to a ConnectedEndpoint. + // be called before dropping all references to a ConnectedEndpoint. Release(ctx context.Context) // CloseUnread sets the fact that this end is closed with unread data to diff --git a/test/syscalls/linux/socket_unix_cmsg.cc b/test/syscalls/linux/socket_unix_cmsg.cc index dc5dcf6b9..a16899493 100644 --- a/test/syscalls/linux/socket_unix_cmsg.cc +++ b/test/syscalls/linux/socket_unix_cmsg.cc @@ -630,9 +630,7 @@ TEST_P(UnixSocketPairCmsgTest, FDPassNotCoalesced) { TransferTest(pair2->first_fd(), pair2->second_fd()); } -// TODO(b/171425923): Enable random/cooperative save once fixed. -TEST_P(UnixSocketPairCmsgTest, FDPassPeek_NoRandomSave) { - const DisableSave ds; +TEST_P(UnixSocketPairCmsgTest, FDPassPeek) { auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); char sent_data[20];