diff options
author | Dean Deng <deandeng@google.com> | 2020-01-28 18:43:24 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-01-28 18:46:14 -0800 |
commit | 4cb55a7a3b09c430fa2b7197fdc7b84b7e88a6ed (patch) | |
tree | c0e7c7bfbed998608fe23dd4e2a7e46f5deaa1b8 /pkg/sentry/socket | |
parent | 396c574db276ae1424af7098b5cd917e2bed9921 (diff) |
Prevent arbitrary size allocation when sending UDS messages.
Currently, Send() will copy data into a new byte slice without regard to the
original size. Size checks should be performed before the allocation takes
place.
Note that for the sake of performance, we avoid putting the buffer
allocation into the critical section. As a result, the size checks need to be
performed again within Enqueue() in case the limit has changed.
PiperOrigin-RevId: 292058147
Diffstat (limited to 'pkg/sentry/socket')
-rw-r--r-- | pkg/sentry/socket/unix/transport/queue.go | 40 | ||||
-rw-r--r-- | pkg/sentry/socket/unix/transport/unix.go | 29 |
2 files changed, 40 insertions, 29 deletions
diff --git a/pkg/sentry/socket/unix/transport/queue.go b/pkg/sentry/socket/unix/transport/queue.go index 5dcd3d95e..d8f3ad63d 100644 --- a/pkg/sentry/socket/unix/transport/queue.go +++ b/pkg/sentry/socket/unix/transport/queue.go @@ -18,6 +18,8 @@ import ( "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/syserr" + "gvisor.dev/gvisor/pkg/tcpip" + "gvisor.dev/gvisor/pkg/tcpip/buffer" "gvisor.dev/gvisor/pkg/waiter" ) @@ -100,12 +102,16 @@ func (q *queue) IsWritable() bool { // Enqueue adds an entry to the data queue if room is available. // +// If discardEmpty is true and there are zero bytes of data, the packet is +// dropped. +// // If truncate is true, Enqueue may truncate the message before enqueuing it. -// Otherwise, the entire message must fit. If n < e.Length(), err indicates why. +// Otherwise, the entire message must fit. If l is less than the size of data, +// err indicates why. // // If notify is true, ReaderQueue.Notify must be called: // q.ReaderQueue.Notify(waiter.EventIn) -func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *syserr.Error) { +func (q *queue) Enqueue(data [][]byte, c ControlMessages, from tcpip.FullAddress, discardEmpty bool, truncate bool) (l int64, notify bool, err *syserr.Error) { q.mu.Lock() if q.closed { @@ -113,9 +119,16 @@ func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *s return 0, false, syserr.ErrClosedForSend } - free := q.limit - q.used + for _, d := range data { + l += int64(len(d)) + } + if discardEmpty && l == 0 { + q.mu.Unlock() + c.Release() + return 0, false, nil + } - l = e.Length() + free := q.limit - q.used if l > free && truncate { if free == 0 { @@ -124,8 +137,7 @@ func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *s return 0, false, syserr.ErrWouldBlock } - e.Truncate(free) - l = e.Length() + l = free err = syserr.ErrWouldBlock } @@ -136,14 +148,26 @@ func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *s } if l > free { - // Message can't fit right now. + // Message can't fit right now, and could not be truncated. q.mu.Unlock() return 0, false, syserr.ErrWouldBlock } + // Aggregate l bytes of data. This will truncate the data if l is less than + // the total bytes held in data. + v := make([]byte, l) + for i, b := 0, v; i < len(data) && len(b) > 0; i++ { + n := copy(b, data[i]) + b = b[n:] + } + notify = q.dataList.Front() == nil q.used += l - q.dataList.PushBack(e) + q.dataList.PushBack(&message{ + Data: buffer.View(v), + Control: c, + Address: from, + }) q.mu.Unlock() diff --git a/pkg/sentry/socket/unix/transport/unix.go b/pkg/sentry/socket/unix/transport/unix.go index dcbafe0e5..2ef654235 100644 --- a/pkg/sentry/socket/unix/transport/unix.go +++ b/pkg/sentry/socket/unix/transport/unix.go @@ -581,7 +581,7 @@ type ConnectedEndpoint interface { // // syserr.ErrWouldBlock can be returned along with a partial write if // the caller should block to send the rest of the data. - Send(data [][]byte, controlMessages ControlMessages, from tcpip.FullAddress) (n int64, notify bool, err *syserr.Error) + Send(data [][]byte, c ControlMessages, from tcpip.FullAddress) (n int64, notify bool, err *syserr.Error) // SendNotify notifies the ConnectedEndpoint of a successful Send. This // must not be called while holding any endpoint locks. @@ -653,35 +653,22 @@ func (e *connectedEndpoint) GetLocalAddress() (tcpip.FullAddress, *tcpip.Error) } // Send implements ConnectedEndpoint.Send. -func (e *connectedEndpoint) Send(data [][]byte, controlMessages ControlMessages, from tcpip.FullAddress) (int64, bool, *syserr.Error) { - var l int64 - for _, d := range data { - l += int64(len(d)) - } - +func (e *connectedEndpoint) Send(data [][]byte, c ControlMessages, from tcpip.FullAddress) (int64, bool, *syserr.Error) { + discardEmpty := false truncate := false if e.endpoint.Type() == linux.SOCK_STREAM { - // Since stream sockets don't preserve message boundaries, we - // can write only as much of the message as fits in the queue. - truncate = true - // Discard empty stream packets. Since stream sockets don't // preserve message boundaries, sending zero bytes is a no-op. // In Linux, the receiver actually uses a zero-length receive // as an indication that the stream was closed. - if l == 0 { - controlMessages.Release() - return 0, false, nil - } - } + discardEmpty = true - v := make([]byte, 0, l) - for _, d := range data { - v = append(v, d...) + // Since stream sockets don't preserve message boundaries, we + // can write only as much of the message as fits in the queue. + truncate = true } - l, notify, err := e.writeQueue.Enqueue(&message{Data: buffer.View(v), Control: controlMessages, Address: from}, truncate) - return int64(l), notify, err + return e.writeQueue.Enqueue(data, c, from, discardEmpty, truncate) } // SendNotify implements ConnectedEndpoint.SendNotify. |