Skip to content

Commit 4cb55a7

Browse files
Dean Denggvisor-bot
Dean Deng
authored andcommitted
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
1 parent 396c574 commit 4cb55a7

File tree

2 files changed

+40
-29
lines changed

2 files changed

+40
-29
lines changed

pkg/sentry/socket/unix/transport/queue.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"gvisor.dev/gvisor/pkg/refs"
1919
"gvisor.dev/gvisor/pkg/sync"
2020
"gvisor.dev/gvisor/pkg/syserr"
21+
"gvisor.dev/gvisor/pkg/tcpip"
22+
"gvisor.dev/gvisor/pkg/tcpip/buffer"
2123
"gvisor.dev/gvisor/pkg/waiter"
2224
)
2325

@@ -100,22 +102,33 @@ func (q *queue) IsWritable() bool {
100102

101103
// Enqueue adds an entry to the data queue if room is available.
102104
//
105+
// If discardEmpty is true and there are zero bytes of data, the packet is
106+
// dropped.
107+
//
103108
// If truncate is true, Enqueue may truncate the message before enqueuing it.
104-
// Otherwise, the entire message must fit. If n < e.Length(), err indicates why.
109+
// Otherwise, the entire message must fit. If l is less than the size of data,
110+
// err indicates why.
105111
//
106112
// If notify is true, ReaderQueue.Notify must be called:
107113
// q.ReaderQueue.Notify(waiter.EventIn)
108-
func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *syserr.Error) {
114+
func (q *queue) Enqueue(data [][]byte, c ControlMessages, from tcpip.FullAddress, discardEmpty bool, truncate bool) (l int64, notify bool, err *syserr.Error) {
109115
q.mu.Lock()
110116

111117
if q.closed {
112118
q.mu.Unlock()
113119
return 0, false, syserr.ErrClosedForSend
114120
}
115121

116-
free := q.limit - q.used
122+
for _, d := range data {
123+
l += int64(len(d))
124+
}
125+
if discardEmpty && l == 0 {
126+
q.mu.Unlock()
127+
c.Release()
128+
return 0, false, nil
129+
}
117130

118-
l = e.Length()
131+
free := q.limit - q.used
119132

120133
if l > free && truncate {
121134
if free == 0 {
@@ -124,8 +137,7 @@ func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *s
124137
return 0, false, syserr.ErrWouldBlock
125138
}
126139

127-
e.Truncate(free)
128-
l = e.Length()
140+
l = free
129141
err = syserr.ErrWouldBlock
130142
}
131143

@@ -136,14 +148,26 @@ func (q *queue) Enqueue(e *message, truncate bool) (l int64, notify bool, err *s
136148
}
137149

138150
if l > free {
139-
// Message can't fit right now.
151+
// Message can't fit right now, and could not be truncated.
140152
q.mu.Unlock()
141153
return 0, false, syserr.ErrWouldBlock
142154
}
143155

156+
// Aggregate l bytes of data. This will truncate the data if l is less than
157+
// the total bytes held in data.
158+
v := make([]byte, l)
159+
for i, b := 0, v; i < len(data) && len(b) > 0; i++ {
160+
n := copy(b, data[i])
161+
b = b[n:]
162+
}
163+
144164
notify = q.dataList.Front() == nil
145165
q.used += l
146-
q.dataList.PushBack(e)
166+
q.dataList.PushBack(&message{
167+
Data: buffer.View(v),
168+
Control: c,
169+
Address: from,
170+
})
147171

148172
q.mu.Unlock()
149173

pkg/sentry/socket/unix/transport/unix.go

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ type ConnectedEndpoint interface {
581581
//
582582
// syserr.ErrWouldBlock can be returned along with a partial write if
583583
// the caller should block to send the rest of the data.
584-
Send(data [][]byte, controlMessages ControlMessages, from tcpip.FullAddress) (n int64, notify bool, err *syserr.Error)
584+
Send(data [][]byte, c ControlMessages, from tcpip.FullAddress) (n int64, notify bool, err *syserr.Error)
585585

586586
// SendNotify notifies the ConnectedEndpoint of a successful Send. This
587587
// must not be called while holding any endpoint locks.
@@ -653,35 +653,22 @@ func (e *connectedEndpoint) GetLocalAddress() (tcpip.FullAddress, *tcpip.Error)
653653
}
654654

655655
// Send implements ConnectedEndpoint.Send.
656-
func (e *connectedEndpoint) Send(data [][]byte, controlMessages ControlMessages, from tcpip.FullAddress) (int64, bool, *syserr.Error) {
657-
var l int64
658-
for _, d := range data {
659-
l += int64(len(d))
660-
}
661-
656+
func (e *connectedEndpoint) Send(data [][]byte, c ControlMessages, from tcpip.FullAddress) (int64, bool, *syserr.Error) {
657+
discardEmpty := false
662658
truncate := false
663659
if e.endpoint.Type() == linux.SOCK_STREAM {
664-
// Since stream sockets don't preserve message boundaries, we
665-
// can write only as much of the message as fits in the queue.
666-
truncate = true
667-
668660
// Discard empty stream packets. Since stream sockets don't
669661
// preserve message boundaries, sending zero bytes is a no-op.
670662
// In Linux, the receiver actually uses a zero-length receive
671663
// as an indication that the stream was closed.
672-
if l == 0 {
673-
controlMessages.Release()
674-
return 0, false, nil
675-
}
676-
}
664+
discardEmpty = true
677665

678-
v := make([]byte, 0, l)
679-
for _, d := range data {
680-
v = append(v, d...)
666+
// Since stream sockets don't preserve message boundaries, we
667+
// can write only as much of the message as fits in the queue.
668+
truncate = true
681669
}
682670

683-
l, notify, err := e.writeQueue.Enqueue(&message{Data: buffer.View(v), Control: controlMessages, Address: from}, truncate)
684-
return int64(l), notify, err
671+
return e.writeQueue.Enqueue(data, c, from, discardEmpty, truncate)
685672
}
686673

687674
// SendNotify implements ConnectedEndpoint.SendNotify.

0 commit comments

Comments
 (0)