Fix (most of) tcp_socket_test on hostinet.

A few fixes in here:

* Linux does not transition the state of a non-blocking socket to SS_CONNECTED
  when the connect happens asynchronously. Instead, it leaves the state as
  SS_CONNECTING (seems like a Linux bug, but OK). This can introduce weird
  behavior for subsequent connect() calls. gVisor now forces the state to
  update to SS_CONNECTED by calling connect() twice.

* Socket shutdown events are slightly different between gVisor and linux. We
  already assert different behavior between the two environments, and now we
  also have to check for gVisor+hostinet instead of just gvisor.

* Handle send/recv timeouts, which fixes blocking for read()/write() syscalls.

* Handle cases like MSG_PEEK where the senderAddress is not returned.

* Pass through some more socketopts, and allow them in the syscall filters.

Something is still wrong with shutdown() and poll() calls, so those tests are
temporarily disabled. I'm looking at those next.

This also fixes a number of other syscall test suites, so those are enabled as
well.

PiperOrigin-RevId: 508397317
This commit is contained in:
Nicolas Lacasse
2023-02-09 09:39:26 -08:00
committed by gVisor bot
parent 60bae95f0a
commit 753fb9ac5e
6 changed files with 152 additions and 37 deletions
+72 -15
View File
@@ -16,6 +16,7 @@ package hostinet
import (
"fmt"
"time"
"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/abi/linux"
@@ -138,8 +139,8 @@ func (s *Socket) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.Read
}
reader := hostfd.GetReadWriterAt(int32(s.fd), -1, opts.Flags)
defer hostfd.PutReadWriterAt(reader)
n, err := dst.CopyOutFrom(ctx, reader)
hostfd.PutReadWriterAt(reader)
return int64(n), err
}
@@ -157,8 +158,8 @@ func (s *Socket) Write(ctx context.Context, src usermem.IOSequence, opts vfs.Wri
}
writer := hostfd.GetReadWriterAt(int32(s.fd), -1, opts.Flags)
defer hostfd.PutReadWriterAt(writer)
n, err := src.CopyInTo(ctx, writer)
hostfd.PutReadWriterAt(writer)
return int64(n), err
}
@@ -244,10 +245,14 @@ func (s *Socket) Connect(t *kernel.Task, sockaddr []byte, blocking bool) *syserr
}
_, _, errno := unix.Syscall(unix.SYS_CONNECT, uintptr(s.fd), uintptr(firstBytePtr(sockaddr)), uintptr(len(sockaddr)))
if errno == 0 {
return nil
}
// The host socket is always non-blocking, so we expect connect to
// return EINPROGRESS. If we are emulating a blocking socket, we will
// wait for the connect to complete below.
// But if we are not emulating a blocking socket, or if we got some
// other error, then return it now.
if errno != unix.EINPROGRESS || !blocking {
return syserr.FromError(translateIOSyscallError(errno))
}
@@ -268,6 +273,7 @@ func (s *Socket) Connect(t *kernel.Task, sockaddr []byte, blocking bool) *syserr
return syserr.FromError(err)
}
}
val, err := unix.GetsockoptInt(s.fd, unix.SOL_SOCKET, unix.SO_ERROR)
if err != nil {
return syserr.FromError(err)
@@ -275,6 +281,22 @@ func (s *Socket) Connect(t *kernel.Task, sockaddr []byte, blocking bool) *syserr
if val != 0 {
return syserr.FromError(unix.Errno(uintptr(val)))
}
// It seems like we are all good now, but Linux has left the socket
// state as CONNECTING (not CONNECTED). This is a strange quirk of
// non-blocking sockets. See tcp_finish_connect() which sets tcp state
// but not socket state.
//
// Sockets in the CONNECTING state can call connect() a second time,
// whereas CONNECTED sockets will reject the second connect() call.
// Because we are emulating a blocking socket, we want a subsequent
// connect() call to fail. So we must kick Linux to update the socket
// to state CONNECTED, which we can do by calling connect() a second
// time ourselves.
_, _, errno = unix.Syscall(unix.SYS_CONNECT, uintptr(s.fd), uintptr(firstBytePtr(sockaddr)), uintptr(len(sockaddr)))
if errno != 0 {
return syserr.FromError(translateIOSyscallError(errno))
}
return nil
}
@@ -297,7 +319,7 @@ func (s *Socket) Accept(t *kernel.Task, peerRequested bool, flags int, blocking
fd, syscallErr := accept4(s.fd, peerAddrPtr, peerAddrlenPtr, unix.SOCK_NONBLOCK|unix.SOCK_CLOEXEC)
if blocking {
var ch chan struct{}
for syscallErr == linuxerr.ErrWouldBlock {
for linuxerr.Equals(linuxerr.ErrWouldBlock, syscallErr) {
if ch != nil {
if syscallErr = t.Block(ch); syscallErr != nil {
break
@@ -367,7 +389,7 @@ func (s *Socket) Shutdown(_ *kernel.Task, how int) *syserr.Error {
}
// GetSockOpt implements socket.Socket.GetSockOpt.
func (s *Socket) GetSockOpt(t *kernel.Task, level int, name int, optValAddr hostarch.Addr, outLen int) (marshal.Marshallable, *syserr.Error) {
func (s *Socket) GetSockOpt(t *kernel.Task, level, name int, optValAddr hostarch.Addr, outLen int) (marshal.Marshallable, *syserr.Error) {
if outLen < 0 {
return nil, syserr.ErrInvalidArgument
}
@@ -387,16 +409,22 @@ func (s *Socket) GetSockOpt(t *kernel.Task, level int, name int, optValAddr host
}
case linux.SOL_SOCKET:
switch name {
case linux.SO_BROADCAST, linux.SO_ERROR, linux.SO_KEEPALIVE, linux.SO_SNDBUF, linux.SO_RCVBUF, linux.SO_REUSEADDR, linux.SO_TIMESTAMP:
case linux.SO_BROADCAST, linux.SO_ERROR, linux.SO_KEEPALIVE, linux.SO_SNDBUF, linux.SO_RCVBUF, linux.SO_REUSEADDR, linux.SO_TIMESTAMP, linux.SO_ACCEPTCONN:
optlen = sizeofInt32
case linux.SO_LINGER:
optlen = unix.SizeofLinger
case linux.SO_RCVTIMEO, linux.SO_SNDTIMEO:
case linux.SO_RCVTIMEO:
optlen = linux.SizeOfTimeval
recvTimeout := linux.NsecToTimeval(s.RecvTimeout())
return &recvTimeout, nil
case linux.SO_SNDTIMEO:
optlen = linux.SizeOfTimeval
sndTimeout := linux.NsecToTimeval(s.SendTimeout())
return &sndTimeout, nil
}
case linux.SOL_TCP:
switch name {
case linux.TCP_NODELAY, linux.TCP_MAXSEG:
case linux.TCP_NODELAY, linux.TCP_MAXSEG, linux.TCP_INQ, linux.TCP_USER_TIMEOUT, linux.TCP_DEFER_ACCEPT, linux.TCP_SYNCNT, linux.TCP_WINDOW_CLAMP:
optlen = sizeofInt32
case linux.TCP_INFO:
optlen = linux.SizeOfTCPInfo
@@ -436,7 +464,7 @@ func (s *Socket) GetSockOpt(t *kernel.Task, level int, name int, optValAddr host
}
// SetSockOpt implements socket.Socket.SetSockOpt.
func (s *Socket) SetSockOpt(t *kernel.Task, level int, name int, opt []byte) *syserr.Error {
func (s *Socket) SetSockOpt(t *kernel.Task, level, name int, opt []byte) *syserr.Error {
// Only allow known and safe options.
optlen := setSockOptLen(t, level, name)
switch level {
@@ -454,10 +482,33 @@ func (s *Socket) SetSockOpt(t *kernel.Task, level int, name int, opt []byte) *sy
switch name {
case linux.SO_BROADCAST, linux.SO_SNDBUF, linux.SO_RCVBUF, linux.SO_REUSEADDR, linux.SO_TIMESTAMP:
optlen = sizeofInt32
case linux.SO_RCVTIMEO:
// Since our host sockets are always non-blocking,
// there is no point in setting these timeouts on the
// host. But we must store them internally so that we
// can put deadlines on our own blocking.
optlen = linux.SizeOfTimeval
var v linux.Timeval
v.UnmarshalBytes(opt[:optlen])
if v.Usec < 0 || v.Usec >= int64(time.Second/time.Microsecond) {
return syserr.ErrDomain
}
s.SetRecvTimeout(v.ToNsecCapped())
return nil
case linux.SO_SNDTIMEO:
// See above.
optlen = linux.SizeOfTimeval
var v linux.Timeval
v.UnmarshalBytes(opt[:optlen])
if v.Usec < 0 || v.Usec >= int64(time.Second/time.Microsecond) {
return syserr.ErrDomain
}
s.SetSendTimeout(v.ToNsecCapped())
return nil
}
case linux.SOL_TCP:
switch name {
case linux.TCP_NODELAY, linux.TCP_INQ, linux.TCP_MAXSEG:
case linux.TCP_NODELAY, linux.TCP_INQ, linux.TCP_MAXSEG, linux.TCP_USER_TIMEOUT, linux.TCP_DEFER_ACCEPT, linux.TCP_SYNCNT, linux.TCP_WINDOW_CLAMP:
optlen = sizeofInt32
case linux.TCP_CONGESTION:
optlen = len(opt)
@@ -473,7 +524,6 @@ func (s *Socket) SetSockOpt(t *kernel.Task, level int, name int, opt []byte) *sy
return syserr.ErrInvalidArgument
}
opt = opt[:optlen]
_, _, errno := unix.Syscall6(unix.SYS_SETSOCKOPT, uintptr(s.fd), uintptr(level), uintptr(name), uintptr(firstBytePtr(opt)), uintptr(len(opt)), 0)
if errno != 0 {
return syserr.FromError(errno)
@@ -515,7 +565,7 @@ func (s *Socket) recvMsgFromHost(iovs []unix.Iovec, flags int, senderRequested b
// RecvMsg implements socket.Socket.RecvMsg.
func (s *Socket) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, haveDeadline bool, deadline ktime.Time, senderRequested bool, controlLen uint64) (int, int, linux.SockAddr, uint32, socket.ControlMessages, *syserr.Error) {
// Only allow known and safe flags.
if flags&^(unix.MSG_DONTWAIT|unix.MSG_PEEK|unix.MSG_TRUNC|unix.MSG_ERRQUEUE) != 0 {
if flags&^(unix.MSG_DONTWAIT|unix.MSG_PEEK|unix.MSG_TRUNC|unix.MSG_CTRUNC|unix.MSG_ERRQUEUE) != 0 {
return 0, 0, nil, 0, socket.ControlMessages{}, syserr.ErrInvalidArgument
}
@@ -549,9 +599,10 @@ func (s *Socket) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, have
var ch chan struct{}
n, err := copyToDst()
// recv*(MSG_ERRQUEUE) never blocks, even without MSG_DONTWAIT.
if flags&(unix.MSG_DONTWAIT|unix.MSG_ERRQUEUE) == 0 {
for err == linuxerr.ErrWouldBlock {
for linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
// We only expect blocking to come from the actual syscall, in which
// case it can't have returned any data.
if n != 0 {
@@ -559,6 +610,9 @@ func (s *Socket) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, have
}
if ch != nil {
if err = t.BlockWithDeadline(ch, haveDeadline, deadline); err != nil {
if linuxerr.Equals(linuxerr.ETIMEDOUT, err) {
err = linuxerr.ErrWouldBlock
}
break
}
} else {
@@ -574,8 +628,11 @@ func (s *Socket) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, have
return 0, 0, nil, 0, socket.ControlMessages{}, syserr.FromError(err)
}
// In some circumstances (like MSG_PEEK specified), the sender address
// field is purposefully ignored. recvMsgFromHost will return an empty
// senderAddrBuf in those cases.
var senderAddr linux.SockAddr
if senderRequested {
if senderRequested && len(senderAddrBuf) > 0 {
senderAddr = socket.UnmarshalSockAddr(s.family, senderAddrBuf)
}
@@ -741,7 +798,7 @@ func (s *Socket) SendMsg(t *kernel.Task, src usermem.IOSequence, to []byte, flag
var ch chan struct{}
n, err := src.CopyInTo(t, sendmsgFromBlocks)
if flags&unix.MSG_DONTWAIT == 0 {
for err == linuxerr.ErrWouldBlock {
for linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
// We only expect blocking to come from the actual syscall, in which
// case it can't have returned any data.
if n != 0 {
+1
View File
@@ -544,6 +544,7 @@ var sockOptNames = map[uint64]abi.ValueSet{
linux.SO_RCVTIMEO: "SO_RCVTIMEO",
linux.SO_OOBINLINE: "SO_OOBINLINE",
linux.SO_TIMESTAMP: "SO_TIMESTAMP",
linux.SO_ACCEPTCONN: "SO_ACCEPTCONN",
},
linux.SOL_TCP: {
linux.TCP_NODELAY: "TCP_NODELAY",
+8 -8
View File
@@ -91,7 +91,7 @@ func Readv(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall
func read(t *kernel.Task, file *vfs.FileDescription, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) {
n, err := file.Read(t, dst, opts)
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
return n, err
}
@@ -115,7 +115,7 @@ func read(t *kernel.Task, file *vfs.FileDescription, dst usermem.IOSequence, opt
// "would block".
n, err = file.Read(t, dst, opts)
total += n
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
break
}
@@ -248,7 +248,7 @@ func Preadv2(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysca
func pread(t *kernel.Task, file *vfs.FileDescription, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) {
n, err := file.PRead(t, dst, offset, opts)
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
return n, err
}
@@ -271,7 +271,7 @@ func pread(t *kernel.Task, file *vfs.FileDescription, dst usermem.IOSequence, of
// "would block".
n, err = file.PRead(t, dst, offset+total, opts)
total += n
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
break
}
@@ -345,7 +345,7 @@ func Writev(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscal
func write(t *kernel.Task, file *vfs.FileDescription, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) {
n, err := file.Write(t, src, opts)
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
return n, err
}
@@ -369,7 +369,7 @@ func write(t *kernel.Task, file *vfs.FileDescription, src usermem.IOSequence, op
// "would block".
n, err = file.Write(t, src, opts)
total += n
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
break
}
@@ -501,7 +501,7 @@ func Pwritev2(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysc
func pwrite(t *kernel.Task, file *vfs.FileDescription, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) {
n, err := file.PWrite(t, src, offset, opts)
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
return n, err
}
@@ -525,7 +525,7 @@ func pwrite(t *kernel.Task, file *vfs.FileDescription, src usermem.IOSequence, o
// "would block".
n, err = file.PWrite(t, src, offset+total, opts)
total += n
if err != linuxerr.ErrWouldBlock {
if !linuxerr.Equals(linuxerr.ErrWouldBlock, err) {
break
}
+41 -6
View File
@@ -164,12 +164,7 @@ func hostInetFilters() seccomp.SyscallRules {
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_SOCKET),
seccomp.EqualTo(unix.SO_RCVTIMEO),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_SOCKET),
seccomp.EqualTo(unix.SO_SNDTIMEO),
seccomp.EqualTo(unix.SO_ACCEPTCONN),
},
{
seccomp.MatchAny{},
@@ -196,6 +191,26 @@ func hostInetFilters() seccomp.SyscallRules {
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_CONGESTION),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_USER_TIMEOUT),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_DEFER_ACCEPT),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_SYNCNT),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_WINDOW_CLAMP),
},
},
unix.SYS_IOCTL: []seccomp.Rule{
{
@@ -283,6 +298,26 @@ func hostInetFilters() seccomp.SyscallRules {
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_CONGESTION),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_USER_TIMEOUT),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_DEFER_ACCEPT),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_SYNCNT),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_TCP),
seccomp.EqualTo(linux.TCP_WINDOW_CLAMP),
},
{
seccomp.MatchAny{},
seccomp.EqualTo(unix.SOL_IP),
+9
View File
@@ -10,11 +10,13 @@ syscall_test(
)
syscall_test(
add_hostinet = True,
test = "//test/syscalls/linux:accept_bind_stream_test",
)
syscall_test(
size = "large",
add_hostinet = True,
shard_count = most_shards,
test = "//test/syscalls/linux:accept_bind_test",
)
@@ -49,6 +51,7 @@ syscall_test(
syscall_test(
size = "large",
add_hostinet = True,
add_overlay = True,
test = "//test/syscalls/linux:bind_test",
)
@@ -111,6 +114,7 @@ syscall_test(
syscall_test(
add_host_communication = True,
add_hostinet = True,
one_sandbox = False,
test = "//test/syscalls/linux:connect_external_test",
# Shared mode tests replace /tmp which hides the files created for
@@ -409,6 +413,7 @@ syscall_test(
)
syscall_test(
add_hostinet = True,
test = "//test/syscalls/linux:partial_bad_buffer_test",
)
@@ -553,6 +558,7 @@ syscall_test(
syscall_test(
size = "medium",
add_hostinet = True,
shard_count = more_shards,
test = "//test/syscalls/linux:readv_socket_test",
)
@@ -609,6 +615,7 @@ syscall_test(
)
syscall_test(
add_hostinet = True,
add_overlay = True,
test = "//test/syscalls/linux:sendfile_socket_test",
)
@@ -869,6 +876,7 @@ syscall_test(
)
syscall_test(
add_hostinet = True,
test = "//test/syscalls/linux:socket_test",
)
@@ -982,6 +990,7 @@ syscall_test(
syscall_test(
size = "medium",
add_hostinet = True,
shard_count = more_shards,
test = "//test/syscalls/linux:tcp_socket_test",
)
+21 -8
View File
@@ -200,6 +200,7 @@ TEST_P(TcpSocketTest, ConnectOnEstablishedConnection) {
connected_.get(),
reinterpret_cast<const struct sockaddr*>(&addr), addrlen),
SyscallFailsWithErrno(EISCONN));
ASSERT_THAT(RetryEINTR(connect)(
accepted_.get(),
reinterpret_cast<const struct sockaddr*>(&addr), addrlen),
@@ -1245,7 +1246,7 @@ TEST_P(SimpleTcpSocketTest, SelfConnectSend) {
SyscallSucceeds());
// Ensure the write buffer is large enough not to block on a single write.
size_t write_size = 512 << 10; // 512 KiB.
size_t write_size = 128 << 10; // 128 KiB.
EXPECT_THAT(setsockopt(s.get(), SOL_SOCKET, SO_SNDBUF, &write_size,
sizeof(write_size)),
SyscallSucceedsWithValue(0));
@@ -2209,14 +2210,20 @@ void ShutdownConnectingSocket(int domain, int shutdown_mode) {
}
TEST_P(SimpleTcpSocketTest, ShutdownReadConnectingSocket) {
// TODO(b/175409607): Fix this test for hostinet.
SKIP_IF(IsRunningWithHostinet());
ShutdownConnectingSocket(GetParam(), SHUT_RD);
}
TEST_P(SimpleTcpSocketTest, ShutdownWriteConnectingSocket) {
// TODO(b/175409607): Fix this test for hostinet.
SKIP_IF(IsRunningWithHostinet());
ShutdownConnectingSocket(GetParam(), SHUT_WR);
}
TEST_P(SimpleTcpSocketTest, ShutdownReadWriteConnectingSocket) {
// TODO(b/175409607): Fix this test for hostinet.
SKIP_IF(IsRunningWithHostinet());
ShutdownConnectingSocket(GetParam(), SHUT_RDWR);
}
@@ -2242,6 +2249,9 @@ TEST_P(SimpleTcpSocketTest, ConnectUnspecifiedAddress) {
}
TEST_P(SimpleTcpSocketTest, OnlyAcknowledgeBacklogConnections) {
// TODO(b/175409607): Fix this test for hostinet.
SKIP_IF(IsRunningWithHostinet());
// At some point, there was a bug in gVisor where a connection could be
// SYN-ACK'd by the server even if the accept queue was already full. This was
// possible because once the listener would process an ACK, it would move the
@@ -2380,26 +2390,29 @@ TEST_P(SimpleTcpSocketTest, SynRcvdOnListenerShutdown) {
const int expected_revents =
POLLIN | POLLOUT | POLLHUP | POLLRDNORM | POLLWRNORM;
// TODO(gvisor.dev/issue/6666): POLLERR is still present
// after getsockopt(..., SO_ERROR, ...) call.
// after getsockopt(..., SO_ERROR, ...) call (unless
// hostinet is used).
if (IsRunningOnGvisor()) {
if (IsRunningWithHostinet()) {
return expected_revents;
}
return expected_revents | POLLPRI | POLLERR;
} else {
return expected_revents | POLLRDHUP;
}
return expected_revents | POLLRDHUP;
}()
#endif
);
EXPECT_THAT(
// TODO(gvisor.dev/issue/6666): on Linux, POLLERR goes away
// after the getsockopt(..., SO_ERROR, ...) call, but not on
// gVisor (unless hostinet is used).
revents,
::testing::AnyOf(
// If the error arrived after poll returned.
::testing::Eq(POLLOUT | POLLWRNORM),
::testing::Eq([expected_revents = poll_fd.revents]() -> int {
// TODO(gvisor.dev/issue/6666): on Linux, POLLERR goes away
// after the getsockopt(..., SO_ERROR, ...) call, but not on
// gVisor.
if (IsRunningOnGvisor()) {
if (IsRunningOnGvisor() && !IsRunningWithHostinet()) {
return expected_revents;
}
return expected_revents | POLLERR;