Skip to content

Commit 000fa84

Browse files
iangudgershentubot
authored andcommitted
Fix tcpip.Endpoint.Write contract regarding short writes
* Clarify tcpip.Endpoint.Write contract regarding short writes. * Enforce tcpip.Endpoint.Write contract regarding short writes. * Update relevant users of tcpip.Endpoint.Write. PiperOrigin-RevId: 224377586 Change-Id: I24299ecce902eb11317ee13dae3b8d8a7c5b097d
1 parent 685eaf1 commit 000fa84

File tree

5 files changed

+38
-15
lines changed

5 files changed

+38
-15
lines changed

pkg/sentry/socket/epsocket/epsocket.go

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -323,20 +323,27 @@ func (s *SocketOperations) Write(ctx context.Context, _ *fs.File, src usermem.IO
323323
f := &ioSequencePayload{ctx: ctx, src: src}
324324
n, resCh, err := s.Endpoint.Write(f, tcpip.WriteOptions{})
325325
if err == tcpip.ErrWouldBlock {
326-
return int64(n), syserror.ErrWouldBlock
326+
return 0, syserror.ErrWouldBlock
327327
}
328328

329329
if resCh != nil {
330330
t := ctx.(*kernel.Task)
331331
if err := t.Block(resCh); err != nil {
332-
return int64(n), syserr.FromError(err).ToError()
332+
return 0, syserr.FromError(err).ToError()
333333
}
334334

335335
n, _, err = s.Endpoint.Write(f, tcpip.WriteOptions{})
336-
return int64(n), syserr.TranslateNetstackError(err).ToError()
337336
}
338337

339-
return int64(n), syserr.TranslateNetstackError(err).ToError()
338+
if err != nil {
339+
return 0, syserr.TranslateNetstackError(err).ToError()
340+
}
341+
342+
if int64(n) < src.NumBytes() {
343+
return int64(n), syserror.ErrWouldBlock
344+
}
345+
346+
return int64(n), nil
340347
}
341348

342349
// Readiness returns a mask of ready events for socket s.
@@ -1343,11 +1350,16 @@ func (s *SocketOperations) SendMsg(t *kernel.Task, src usermem.IOSequence, to []
13431350
n, resCh, err := s.Endpoint.Write(tcpip.SlicePayload(v), opts)
13441351
if resCh != nil {
13451352
if err := t.Block(resCh); err != nil {
1346-
return int(n), syserr.FromError(err)
1353+
return 0, syserr.FromError(err)
13471354
}
13481355
n, _, err = s.Endpoint.Write(tcpip.SlicePayload(v), opts)
13491356
}
1350-
if err != tcpip.ErrWouldBlock || flags&linux.MSG_DONTWAIT != 0 {
1357+
dontWait := flags&linux.MSG_DONTWAIT != 0
1358+
if err == nil && (n >= uintptr(len(v)) || dontWait) {
1359+
// Complete write.
1360+
return int(n), nil
1361+
}
1362+
if err != nil && (err != tcpip.ErrWouldBlock || dontWait) {
13511363
return int(n), syserr.TranslateNetstackError(err)
13521364
}
13531365

@@ -1363,11 +1375,17 @@ func (s *SocketOperations) SendMsg(t *kernel.Task, src usermem.IOSequence, to []
13631375
n, _, err = s.Endpoint.Write(tcpip.SlicePayload(v), opts)
13641376
v.TrimFront(int(n))
13651377
total += n
1366-
if err != tcpip.ErrWouldBlock {
1367-
return int(total), syserr.TranslateNetstackError(err)
1378+
1379+
if err != nil && err != tcpip.ErrWouldBlock && total == 0 {
1380+
return 0, syserr.TranslateNetstackError(err)
1381+
}
1382+
1383+
if err == nil && len(v) == 0 || err != nil && err != tcpip.ErrWouldBlock {
1384+
return int(total), nil
13681385
}
13691386

13701387
if err := t.Block(ch); err != nil {
1388+
// handleIOError will consume errors from t.Block if needed.
13711389
return int(total), syserr.FromError(err)
13721390
}
13731391
}

pkg/sentry/socket/socket.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ type Socket interface {
9090

9191
// SendMsg implements the sendmsg(2) linux syscall. SendMsg does not take
9292
// ownership of the ControlMessage on error.
93+
//
94+
// If n > 0, err will either be nil or an error from t.Block.
9395
SendMsg(t *kernel.Task, src usermem.IOSequence, to []byte, flags int, controlMessages ControlMessages) (n int, err *syserr.Error)
9496

9597
// SetRecvTimeout sets the timeout (in ns) for recv operations. Zero means

pkg/tcpip/tcpip.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,10 @@ type Endpoint interface {
312312
// the caller should not use data[:n] after Write returns.
313313
//
314314
// Note that unlike io.Writer.Write, it is not an error for Write to
315-
// perform a partial write.
315+
// perform a partial write (if n > 0, no error may be returned). Only
316+
// stream (TCP) Endpoints may return partial writes, and even then only
317+
// in the case where writing additional data would block. Other Endpoints
318+
// will either write the entire message or return an error.
316319
//
317320
// For UDP and Ping sockets if address resolution is required,
318321
// ErrNoLinkAddress and a notification channel is returned for the caller to

pkg/tcpip/transport/ping/endpoint.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,11 @@ func (e *endpoint) Write(p tcpip.Payload, opts tcpip.WriteOptions) (uintptr, <-c
299299
err = sendPing6(route, e.id.LocalPort, v)
300300
}
301301

302-
return uintptr(len(v)), nil, err
302+
if err != nil {
303+
return 0, nil, err
304+
}
305+
306+
return uintptr(len(v)), nil, nil
303307
}
304308

305309
// Peek only returns data from a single datagram, so do nothing here.

pkg/tcpip/transport/tcp/endpoint.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -554,10 +554,6 @@ func (e *endpoint) Write(p tcpip.Payload, opts tcpip.WriteOptions) (uintptr, <-c
554554
return 0, nil, perr
555555
}
556556

557-
var err *tcpip.Error
558-
if p.Size() > avail {
559-
err = tcpip.ErrWouldBlock
560-
}
561557
l := len(v)
562558
s := newSegmentFromView(&e.route, e.id, v)
563559

@@ -576,7 +572,7 @@ func (e *endpoint) Write(p tcpip.Payload, opts tcpip.WriteOptions) (uintptr, <-c
576572
// Let the protocol goroutine do the work.
577573
e.sndWaker.Assert()
578574
}
579-
return uintptr(l), nil, err
575+
return uintptr(l), nil, nil
580576
}
581577

582578
// Peek reads data without consuming it from the endpoint.

0 commit comments

Comments
 (0)