From d01240d871c8737989b1af27c137f6ae40bc6d37 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 8 Jan 2020 13:52:56 -0800 Subject: Take addresses as const PiperOrigin-RevId: 288767927 --- test/syscalls/linux/ip_socket_test_util.cc | 10 +++++----- test/syscalls/linux/ip_socket_test_util.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'test/syscalls/linux') diff --git a/test/syscalls/linux/ip_socket_test_util.cc b/test/syscalls/linux/ip_socket_test_util.cc index 8398fc95f..6b472eb2f 100644 --- a/test/syscalls/linux/ip_socket_test_util.cc +++ b/test/syscalls/linux/ip_socket_test_util.cc @@ -187,24 +187,24 @@ PosixErrorOr IfAddrHelper::GetIndex(std::string name) { return InterfaceIndex(name); } -std::string GetAddr4Str(in_addr* a) { +std::string GetAddr4Str(const in_addr* a) { char str[INET_ADDRSTRLEN]; inet_ntop(AF_INET, a, str, sizeof(str)); return std::string(str); } -std::string GetAddr6Str(in6_addr* a) { +std::string GetAddr6Str(const in6_addr* a) { char str[INET6_ADDRSTRLEN]; inet_ntop(AF_INET6, a, str, sizeof(str)); return std::string(str); } -std::string GetAddrStr(sockaddr* a) { +std::string GetAddrStr(const sockaddr* a) { if (a->sa_family == AF_INET) { - auto src = &(reinterpret_cast(a)->sin_addr); + auto src = &(reinterpret_cast(a)->sin_addr); return GetAddr4Str(src); } else if (a->sa_family == AF_INET6) { - auto src = &(reinterpret_cast(a)->sin6_addr); + auto src = &(reinterpret_cast(a)->sin6_addr); return GetAddr6Str(src); } return std::string(""); diff --git a/test/syscalls/linux/ip_socket_test_util.h b/test/syscalls/linux/ip_socket_test_util.h index 9cb4566db..0f58e0f77 100644 --- a/test/syscalls/linux/ip_socket_test_util.h +++ b/test/syscalls/linux/ip_socket_test_util.h @@ -105,14 +105,14 @@ class IfAddrHelper { }; // GetAddr4Str returns the given IPv4 network address structure as a string. -std::string GetAddr4Str(in_addr* a); +std::string GetAddr4Str(const in_addr* a); // GetAddr6Str returns the given IPv6 network address structure as a string. -std::string GetAddr6Str(in6_addr* a); +std::string GetAddr6Str(const in6_addr* a); // GetAddrStr returns the given IPv4 or IPv6 network address structure as a // string. -std::string GetAddrStr(sockaddr* a); +std::string GetAddrStr(const sockaddr* a); } // namespace testing } // namespace gvisor -- cgit v1.2.3 From b3ae8a62cfdf13821d35467d4150ed983ac556f1 Mon Sep 17 00:00:00 2001 From: Ting-Yu Wang Date: Wed, 8 Jan 2020 16:29:12 -0800 Subject: Fix slice bounds out of range panic in parsing socket control message. Panic found by syzakller. PiperOrigin-RevId: 288799046 --- pkg/sentry/socket/control/control.go | 6 ++++++ test/syscalls/linux/socket_ip_unbound.cc | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) (limited to 'test/syscalls/linux') diff --git a/pkg/sentry/socket/control/control.go b/pkg/sentry/socket/control/control.go index af1a4e95f..4301b697c 100644 --- a/pkg/sentry/socket/control/control.go +++ b/pkg/sentry/socket/control/control.go @@ -471,6 +471,9 @@ func Parse(t *kernel.Task, socketOrEndpoint interface{}, buf []byte) (socket.Con case linux.SOL_IP: switch h.Type { case linux.IP_TOS: + if length < linux.SizeOfControlMessageTOS { + return socket.ControlMessages{}, syserror.EINVAL + } cmsgs.IP.HasTOS = true binary.Unmarshal(buf[i:i+linux.SizeOfControlMessageTOS], usermem.ByteOrder, &cmsgs.IP.TOS) i += AlignUp(length, width) @@ -481,6 +484,9 @@ func Parse(t *kernel.Task, socketOrEndpoint interface{}, buf []byte) (socket.Con case linux.SOL_IPV6: switch h.Type { case linux.IPV6_TCLASS: + if length < linux.SizeOfControlMessageTClass { + return socket.ControlMessages{}, syserror.EINVAL + } cmsgs.IP.HasTClass = true binary.Unmarshal(buf[i:i+linux.SizeOfControlMessageTClass], usermem.ByteOrder, &cmsgs.IP.TClass) i += AlignUp(length, width) diff --git a/test/syscalls/linux/socket_ip_unbound.cc b/test/syscalls/linux/socket_ip_unbound.cc index b6754111f..ca597e267 100644 --- a/test/syscalls/linux/socket_ip_unbound.cc +++ b/test/syscalls/linux/socket_ip_unbound.cc @@ -129,6 +129,7 @@ TEST_P(IPUnboundSocketTest, InvalidNegativeTtl) { struct TOSOption { int level; int option; + int cmsg_level; }; constexpr int INET_ECN_MASK = 3; @@ -139,10 +140,12 @@ static TOSOption GetTOSOption(int domain) { case AF_INET: opt.level = IPPROTO_IP; opt.option = IP_TOS; + opt.cmsg_level = SOL_IP; break; case AF_INET6: opt.level = IPPROTO_IPV6; opt.option = IPV6_TCLASS; + opt.cmsg_level = SOL_IPV6; break; } return opt; @@ -386,6 +389,36 @@ TEST_P(IPUnboundSocketTest, NullTOS) { SyscallFailsWithErrno(EFAULT)); } +TEST_P(IPUnboundSocketTest, InsufficientBufferTOS) { + SKIP_IF(GetParam().protocol == IPPROTO_TCP); + + auto socket = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); + TOSOption t = GetTOSOption(GetParam().domain); + + in_addr addr4; + in6_addr addr6; + ASSERT_THAT(inet_pton(AF_INET, "127.0.0.1", &addr4), ::testing::Eq(1)); + ASSERT_THAT(inet_pton(AF_INET6, "fe80::", &addr6), ::testing::Eq(1)); + + cmsghdr cmsg = {}; + cmsg.cmsg_len = sizeof(cmsg); + cmsg.cmsg_level = t.cmsg_level; + cmsg.cmsg_type = t.option; + + msghdr msg = {}; + msg.msg_control = &cmsg; + msg.msg_controllen = sizeof(cmsg); + if (GetParam().domain == AF_INET) { + msg.msg_name = &addr4; + msg.msg_namelen = sizeof(addr4); + } else { + msg.msg_name = &addr6; + msg.msg_namelen = sizeof(addr6); + } + + EXPECT_THAT(sendmsg(socket->get(), &msg, 0), SyscallFailsWithErrno(EINVAL)); +} + INSTANTIATE_TEST_SUITE_P( IPUnboundSockets, IPUnboundSocketTest, ::testing::ValuesIn(VecCat(VecCat( -- cgit v1.2.3 From fbb2c008e26a7e9d860f6cbf796ea7c375858502 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Wed, 8 Jan 2020 16:35:43 -0800 Subject: Return correct length with MSG_TRUNC for unix sockets. This change calls a new Truncate method on the EndpointReader in RecvMsg for both netlink and unix sockets. This allows readers such as sockets to peek at the length of data without actually reading it to a buffer. Fixes #993 #1240 PiperOrigin-RevId: 288800167 --- pkg/sentry/socket/netlink/BUILD | 1 - pkg/sentry/socket/netlink/socket.go | 29 +++--- pkg/sentry/socket/unix/io.go | 13 +++ pkg/sentry/socket/unix/unix.go | 23 ++++- test/syscalls/linux/BUILD | 1 - test/syscalls/linux/socket_non_stream.cc | 113 +++++++++++++++++++++- test/syscalls/linux/socket_non_stream_blocking.cc | 37 +++++++ test/syscalls/linux/socket_stream.cc | 55 ++++++++++- 8 files changed, 250 insertions(+), 22 deletions(-) (limited to 'test/syscalls/linux') diff --git a/pkg/sentry/socket/netlink/BUILD b/pkg/sentry/socket/netlink/BUILD index 79589e3c8..136821963 100644 --- a/pkg/sentry/socket/netlink/BUILD +++ b/pkg/sentry/socket/netlink/BUILD @@ -22,7 +22,6 @@ go_library( "//pkg/sentry/kernel", "//pkg/sentry/kernel/auth", "//pkg/sentry/kernel/time", - "//pkg/sentry/safemem", "//pkg/sentry/socket", "//pkg/sentry/socket/netlink/port", "//pkg/sentry/socket/unix", diff --git a/pkg/sentry/socket/netlink/socket.go b/pkg/sentry/socket/netlink/socket.go index 4a1b87a9a..d2e3644a6 100644 --- a/pkg/sentry/socket/netlink/socket.go +++ b/pkg/sentry/socket/netlink/socket.go @@ -29,7 +29,6 @@ import ( "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" ktime "gvisor.dev/gvisor/pkg/sentry/kernel/time" - "gvisor.dev/gvisor/pkg/sentry/safemem" "gvisor.dev/gvisor/pkg/sentry/socket" "gvisor.dev/gvisor/pkg/sentry/socket/netlink/port" "gvisor.dev/gvisor/pkg/sentry/socket/unix" @@ -500,29 +499,29 @@ func (s *Socket) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, have trunc := flags&linux.MSG_TRUNC != 0 r := unix.EndpointReader{ + Ctx: t, Endpoint: s.ep, Peek: flags&linux.MSG_PEEK != 0, } + doRead := func() (int64, error) { + return dst.CopyOutFrom(t, &r) + } + // If MSG_TRUNC is set with a zero byte destination then we still need // to read the message and discard it, or in the case where MSG_PEEK is // set, leave it be. In both cases the full message length must be - // returned. However, the memory manager for the destination will not read - // the endpoint if the destination is zero length. - // - // In order for the endpoint to be read when the destination size is zero, - // we must cause a read of the endpoint by using a separate fake zero - // length block sequence and calling the EndpointReader directly. + // returned. if trunc && dst.Addrs.NumBytes() == 0 { - // Perform a read to a zero byte block sequence. We can ignore the - // original destination since it was zero bytes. The length returned by - // ReadToBlocks is ignored and we return the full message length to comply - // with MSG_TRUNC. - _, err := r.ReadToBlocks(safemem.BlockSeqOf(safemem.BlockFromSafeSlice(make([]byte, 0)))) - return int(r.MsgSize), linux.MSG_TRUNC, from, fromLen, socket.ControlMessages{}, syserr.FromError(err) + doRead = func() (int64, error) { + err := r.Truncate() + // Always return zero for bytes read since the destination size is + // zero. + return 0, err + } } - if n, err := dst.CopyOutFrom(t, &r); err != syserror.ErrWouldBlock || flags&linux.MSG_DONTWAIT != 0 { + if n, err := doRead(); err != syserror.ErrWouldBlock || flags&linux.MSG_DONTWAIT != 0 { var mflags int if n < int64(r.MsgSize) { mflags |= linux.MSG_TRUNC @@ -540,7 +539,7 @@ func (s *Socket) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, have defer s.EventUnregister(&e) for { - if n, err := dst.CopyOutFrom(t, &r); err != syserror.ErrWouldBlock { + if n, err := doRead(); err != syserror.ErrWouldBlock { var mflags int if n < int64(r.MsgSize) { mflags |= linux.MSG_TRUNC diff --git a/pkg/sentry/socket/unix/io.go b/pkg/sentry/socket/unix/io.go index 2ec1a662d..2447f24ef 100644 --- a/pkg/sentry/socket/unix/io.go +++ b/pkg/sentry/socket/unix/io.go @@ -83,6 +83,19 @@ type EndpointReader struct { ControlTrunc bool } +// Truncate calls RecvMsg on the endpoint without writing to a destination. +func (r *EndpointReader) Truncate() error { + // Ignore bytes read since it will always be zero. + _, ms, c, ct, err := r.Endpoint.RecvMsg(r.Ctx, [][]byte{}, r.Creds, r.NumRights, r.Peek, r.From) + r.Control = c + r.ControlTrunc = ct + r.MsgSize = ms + if err != nil { + return err.ToError() + } + return nil +} + // ReadToBlocks implements safemem.Reader.ReadToBlocks. func (r *EndpointReader) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) { return safemem.FromVecReaderFunc{func(bufs [][]byte) (int64, error) { diff --git a/pkg/sentry/socket/unix/unix.go b/pkg/sentry/socket/unix/unix.go index 885758054..91effe89a 100644 --- a/pkg/sentry/socket/unix/unix.go +++ b/pkg/sentry/socket/unix/unix.go @@ -544,8 +544,27 @@ func (s *SocketOperations) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags if senderRequested { r.From = &tcpip.FullAddress{} } + + doRead := func() (int64, error) { + return dst.CopyOutFrom(t, &r) + } + + // If MSG_TRUNC is set with a zero byte destination then we still need + // to read the message and discard it, or in the case where MSG_PEEK is + // set, leave it be. In both cases the full message length must be + // returned. + if trunc && dst.Addrs.NumBytes() == 0 { + doRead = func() (int64, error) { + err := r.Truncate() + // Always return zero for bytes read since the destination size is + // zero. + return 0, err + } + + } + var total int64 - if n, err := dst.CopyOutFrom(t, &r); err != syserror.ErrWouldBlock || dontWait { + if n, err := doRead(); err != syserror.ErrWouldBlock || dontWait { var from linux.SockAddr var fromLen uint32 if r.From != nil && len([]byte(r.From.Addr)) != 0 { @@ -580,7 +599,7 @@ func (s *SocketOperations) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags defer s.EventUnregister(&e) for { - if n, err := dst.CopyOutFrom(t, &r); err != syserror.ErrWouldBlock { + if n, err := doRead(); err != syserror.ErrWouldBlock { var from linux.SockAddr var fromLen uint32 if r.From != nil { diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 064ce8429..ce8abe217 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -2888,7 +2888,6 @@ cc_library( ":unix_domain_socket_test_util", "//test/util:test_util", "//test/util:thread_util", - "//test/util:timer_util", "@com_google_absl//absl/time", "@com_google_googletest//:gtest", ], diff --git a/test/syscalls/linux/socket_non_stream.cc b/test/syscalls/linux/socket_non_stream.cc index d91c5ed39..c61817f14 100644 --- a/test/syscalls/linux/socket_non_stream.cc +++ b/test/syscalls/linux/socket_non_stream.cc @@ -113,7 +113,7 @@ TEST_P(NonStreamSocketPairTest, RecvmsgMsghdrFlagMsgTrunc) { EXPECT_EQ(0, memcmp(received_data, sent_data, sizeof(received_data))); // Check that msghdr flags were updated. - EXPECT_EQ(msg.msg_flags, MSG_TRUNC); + EXPECT_EQ(msg.msg_flags & MSG_TRUNC, MSG_TRUNC); } // Stream sockets allow data sent with multiple sends to be peeked at in a @@ -193,7 +193,7 @@ TEST_P(NonStreamSocketPairTest, MsgTruncTruncationRecvmsgMsghdrFlagMsgTrunc) { EXPECT_EQ(0, memcmp(received_data, sent_data, sizeof(received_data))); // Check that msghdr flags were updated. - EXPECT_EQ(msg.msg_flags, MSG_TRUNC); + EXPECT_EQ(msg.msg_flags & MSG_TRUNC, MSG_TRUNC); } TEST_P(NonStreamSocketPairTest, MsgTruncSameSize) { @@ -224,5 +224,114 @@ TEST_P(NonStreamSocketPairTest, MsgTruncNotFull) { EXPECT_EQ(0, memcmp(sent_data, received_data, sizeof(sent_data))); } +// This test tests reading from a socket with MSG_TRUNC and a zero length +// receive buffer. The user should be able to get the message length. +TEST_P(NonStreamSocketPairTest, RecvmsgMsgTruncZeroLen) { + auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); + + char sent_data[10]; + RandomizeBuffer(sent_data, sizeof(sent_data)); + ASSERT_THAT( + RetryEINTR(send)(sockets->first_fd(), sent_data, sizeof(sent_data), 0), + SyscallSucceedsWithValue(sizeof(sent_data))); + + // The receive buffer is of zero length. + char received_data[0] = {}; + + struct iovec iov; + iov.iov_base = received_data; + iov.iov_len = sizeof(received_data); + struct msghdr msg = {}; + msg.msg_flags = -1; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + + // The syscall succeeds returning the full size of the message on the socket. + ASSERT_THAT(RetryEINTR(recvmsg)(sockets->second_fd(), &msg, MSG_TRUNC), + SyscallSucceedsWithValue(sizeof(sent_data))); + + // Check that MSG_TRUNC is set on msghdr flags. + EXPECT_EQ(msg.msg_flags & MSG_TRUNC, MSG_TRUNC); +} + +// This test tests reading from a socket with MSG_TRUNC | MSG_PEEK and a zero +// length receive buffer. The user should be able to get the message length +// without reading data off the socket. +TEST_P(NonStreamSocketPairTest, RecvmsgMsgTruncMsgPeekZeroLen) { + auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); + + char sent_data[10]; + RandomizeBuffer(sent_data, sizeof(sent_data)); + ASSERT_THAT( + RetryEINTR(send)(sockets->first_fd(), sent_data, sizeof(sent_data), 0), + SyscallSucceedsWithValue(sizeof(sent_data))); + + // The receive buffer is of zero length. + char peek_data[0] = {}; + + struct iovec peek_iov; + peek_iov.iov_base = peek_data; + peek_iov.iov_len = sizeof(peek_data); + struct msghdr peek_msg = {}; + peek_msg.msg_flags = -1; + peek_msg.msg_iov = &peek_iov; + peek_msg.msg_iovlen = 1; + + // The syscall succeeds returning the full size of the message on the socket. + ASSERT_THAT(RetryEINTR(recvmsg)(sockets->second_fd(), &peek_msg, + MSG_TRUNC | MSG_PEEK), + SyscallSucceedsWithValue(sizeof(sent_data))); + + // Check that MSG_TRUNC is set on msghdr flags because the receive buffer is + // smaller than the message size. + EXPECT_EQ(peek_msg.msg_flags & MSG_TRUNC, MSG_TRUNC); + + char received_data[sizeof(sent_data)] = {}; + + struct iovec received_iov; + received_iov.iov_base = received_data; + received_iov.iov_len = sizeof(received_data); + struct msghdr received_msg = {}; + received_msg.msg_flags = -1; + received_msg.msg_iov = &received_iov; + received_msg.msg_iovlen = 1; + + // Next we can read the actual data. + ASSERT_THAT( + RetryEINTR(recvmsg)(sockets->second_fd(), &received_msg, MSG_TRUNC), + SyscallSucceedsWithValue(sizeof(sent_data))); + + EXPECT_EQ(0, memcmp(sent_data, received_data, sizeof(sent_data))); + + // Check that MSG_TRUNC is not set on msghdr flags because we read the whole + // message. + EXPECT_EQ(received_msg.msg_flags & MSG_TRUNC, 0); +} + +// This test tests reading from a socket with MSG_TRUNC | MSG_PEEK and a zero +// length receive buffer and MSG_DONTWAIT. The user should be able to get an +// EAGAIN or EWOULDBLOCK error response. +TEST_P(NonStreamSocketPairTest, RecvmsgTruncPeekDontwaitZeroLen) { + auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); + + // NOTE: We don't send any data on the socket. + + // The receive buffer is of zero length. + char peek_data[0] = {}; + + struct iovec peek_iov; + peek_iov.iov_base = peek_data; + peek_iov.iov_len = sizeof(peek_data); + struct msghdr peek_msg = {}; + peek_msg.msg_flags = -1; + peek_msg.msg_iov = &peek_iov; + peek_msg.msg_iovlen = 1; + + // recvmsg fails with EAGAIN because no data is available on the socket. + ASSERT_THAT(RetryEINTR(recvmsg)(sockets->second_fd(), &peek_msg, + MSG_TRUNC | MSG_PEEK | MSG_DONTWAIT), + SyscallFailsWithErrno(EAGAIN)); +} + } // namespace testing } // namespace gvisor diff --git a/test/syscalls/linux/socket_non_stream_blocking.cc b/test/syscalls/linux/socket_non_stream_blocking.cc index 62d87c1af..b052f6e61 100644 --- a/test/syscalls/linux/socket_non_stream_blocking.cc +++ b/test/syscalls/linux/socket_non_stream_blocking.cc @@ -25,6 +25,7 @@ #include "test/syscalls/linux/socket_test_util.h" #include "test/syscalls/linux/unix_domain_socket_test_util.h" #include "test/util/test_util.h" +#include "test/util/thread_util.h" namespace gvisor { namespace testing { @@ -44,5 +45,41 @@ TEST_P(BlockingNonStreamSocketPairTest, RecvLessThanBufferWaitAll) { SyscallSucceedsWithValue(sizeof(sent_data))); } +// This test tests reading from a socket with MSG_TRUNC | MSG_PEEK and a zero +// length receive buffer and MSG_DONTWAIT. The recvmsg call should block on +// reading the data. +TEST_P(BlockingNonStreamSocketPairTest, + RecvmsgTruncPeekDontwaitZeroLenBlocking) { + auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); + + // NOTE: We don't initially send any data on the socket. + const int data_size = 10; + char sent_data[data_size]; + RandomizeBuffer(sent_data, data_size); + + // The receive buffer is of zero length. + char peek_data[0] = {}; + + struct iovec peek_iov; + peek_iov.iov_base = peek_data; + peek_iov.iov_len = sizeof(peek_data); + struct msghdr peek_msg = {}; + peek_msg.msg_flags = -1; + peek_msg.msg_iov = &peek_iov; + peek_msg.msg_iovlen = 1; + + ScopedThread t([&]() { + // The syscall succeeds returning the full size of the message on the + // socket. This should block until there is data on the socket. + ASSERT_THAT(RetryEINTR(recvmsg)(sockets->second_fd(), &peek_msg, + MSG_TRUNC | MSG_PEEK), + SyscallSucceedsWithValue(data_size)); + }); + + absl::SleepFor(absl::Seconds(1)); + ASSERT_THAT(RetryEINTR(send)(sockets->first_fd(), sent_data, data_size, 0), + SyscallSucceedsWithValue(data_size)); +} + } // namespace testing } // namespace gvisor diff --git a/test/syscalls/linux/socket_stream.cc b/test/syscalls/linux/socket_stream.cc index 346443f96..6522b2e01 100644 --- a/test/syscalls/linux/socket_stream.cc +++ b/test/syscalls/linux/socket_stream.cc @@ -104,7 +104,60 @@ TEST_P(StreamSocketPairTest, RecvmsgMsghdrFlagsNoMsgTrunc) { EXPECT_EQ(0, memcmp(received_data, sent_data, sizeof(received_data))); // Check that msghdr flags were cleared (MSG_TRUNC was not set). - EXPECT_EQ(msg.msg_flags, 0); + ASSERT_EQ(msg.msg_flags & MSG_TRUNC, 0); +} + +TEST_P(StreamSocketPairTest, RecvmsgTruncZeroLen) { + auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); + + char sent_data[10]; + RandomizeBuffer(sent_data, sizeof(sent_data)); + ASSERT_THAT( + RetryEINTR(send)(sockets->first_fd(), sent_data, sizeof(sent_data), 0), + SyscallSucceedsWithValue(sizeof(sent_data))); + + char received_data[0] = {}; + + struct iovec iov; + iov.iov_base = received_data; + iov.iov_len = sizeof(received_data); + struct msghdr msg = {}; + msg.msg_flags = -1; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + + ASSERT_THAT(RetryEINTR(recvmsg)(sockets->second_fd(), &msg, MSG_TRUNC), + SyscallSucceedsWithValue(0)); + + // Check that msghdr flags were cleared (MSG_TRUNC was not set). + ASSERT_EQ(msg.msg_flags & MSG_TRUNC, 0); +} + +TEST_P(StreamSocketPairTest, RecvmsgTruncPeekZeroLen) { + auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); + + char sent_data[10]; + RandomizeBuffer(sent_data, sizeof(sent_data)); + ASSERT_THAT( + RetryEINTR(send)(sockets->first_fd(), sent_data, sizeof(sent_data), 0), + SyscallSucceedsWithValue(sizeof(sent_data))); + + char received_data[0] = {}; + + struct iovec iov; + iov.iov_base = received_data; + iov.iov_len = sizeof(received_data); + struct msghdr msg = {}; + msg.msg_flags = -1; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + + ASSERT_THAT( + RetryEINTR(recvmsg)(sockets->second_fd(), &msg, MSG_TRUNC | MSG_PEEK), + SyscallSucceedsWithValue(0)); + + // Check that msghdr flags were cleared (MSG_TRUNC was not set). + ASSERT_EQ(msg.msg_flags & MSG_TRUNC, 0); } TEST_P(StreamSocketPairTest, MsgTrunc) { -- cgit v1.2.3 From 356d81146bafc4b4548163eb87e886c851b49e12 Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Thu, 9 Jan 2020 17:56:58 -0800 Subject: Deflake a couple of TCP syscall tests when run under gotsan. PiperOrigin-RevId: 289010316 --- .../linux/socket_bind_to_device_distribution.cc | 25 ++++++++++++++++++-- test/syscalls/linux/socket_inet_loopback.cc | 27 +++++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) (limited to 'test/syscalls/linux') diff --git a/test/syscalls/linux/socket_bind_to_device_distribution.cc b/test/syscalls/linux/socket_bind_to_device_distribution.cc index 5767181a1..5ed57625c 100644 --- a/test/syscalls/linux/socket_bind_to_device_distribution.cc +++ b/test/syscalls/linux/socket_bind_to_device_distribution.cc @@ -183,7 +183,14 @@ TEST_P(BindToDeviceDistributionTest, Tcp) { } // Receive some data from a socket to be sure that the connect() // system call has been completed on another side. - int data; + // Do a short read and then close the socket to trigger a RST. This + // ensures that both ends of the connection are cleaned up and no + // goroutines hang around in TIME-WAIT. We do this so that this test + // does not timeout under gotsan runs where lots of goroutines can + // cause the test to use absurd amounts of memory. + // + // See: https://tools.ietf.org/html/rfc2525#page-50 section 2.17 + uint16_t data; EXPECT_THAT( RetryEINTR(recv)(fd.ValueOrDie().get(), &data, sizeof(data), 0), SyscallSucceedsWithValue(sizeof(data))); @@ -198,15 +205,29 @@ TEST_P(BindToDeviceDistributionTest, Tcp) { } for (int i = 0; i < kConnectAttempts; i++) { - FileDescriptor const fd = ASSERT_NO_ERRNO_AND_VALUE( + const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE( Socket(connector.family(), SOCK_STREAM, IPPROTO_TCP)); ASSERT_THAT( RetryEINTR(connect)(fd.get(), reinterpret_cast(&conn_addr), connector.addr_len), SyscallSucceeds()); + // Do two separate sends to ensure two segments are received. This is + // required for netstack where read is incorrectly assuming a whole + // segment is read when endpoint.Read() is called which is technically + // incorrect as the syscall that invoked endpoint.Read() may only + // consume it partially. This results in a case where a close() of + // such a socket does not trigger a RST in netstack due to the + // endpoint assuming that the endpoint has no unread data. EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), SyscallSucceedsWithValue(sizeof(i))); + + // TODO(gvisor.dev/issue/1449): Remove this block once netstack correctly + // generates a RST. + if (IsRunningOnGvisor()) { + EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), + SyscallSucceedsWithValue(sizeof(i))); + } } // Join threads to be sure that all connections have been counted. diff --git a/test/syscalls/linux/socket_inet_loopback.cc b/test/syscalls/linux/socket_inet_loopback.cc index 619d41901..138024d9e 100644 --- a/test/syscalls/linux/socket_inet_loopback.cc +++ b/test/syscalls/linux/socket_inet_loopback.cc @@ -714,7 +714,7 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { sockaddr_storage listen_addr = listener.addr; sockaddr_storage conn_addr = connector.addr; constexpr int kThreadCount = 3; - constexpr int kConnectAttempts = 4096; + constexpr int kConnectAttempts = 10000; // Create the listening socket. FileDescriptor listener_fds[kThreadCount]; @@ -729,7 +729,7 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { ASSERT_THAT( bind(fd, reinterpret_cast(&listen_addr), listener.addr_len), SyscallSucceeds()); - ASSERT_THAT(listen(fd, kConnectAttempts / 3), SyscallSucceeds()); + ASSERT_THAT(listen(fd, 40), SyscallSucceeds()); // On the first bind we need to determine which port was bound. if (i != 0) { @@ -772,7 +772,14 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { } // Receive some data from a socket to be sure that the connect() // system call has been completed on another side. - int data; + // Do a short read and then close the socket to trigger a RST. This + // ensures that both ends of the connection are cleaned up and no + // goroutines hang around in TIME-WAIT. We do this so that this test + // does not timeout under gotsan runs where lots of goroutines can + // cause the test to use absurd amounts of memory. + // + // See: https://tools.ietf.org/html/rfc2525#page-50 section 2.17 + uint16_t data; EXPECT_THAT( RetryEINTR(recv)(fd.ValueOrDie().get(), &data, sizeof(data), 0), SyscallSucceedsWithValue(sizeof(data))); @@ -795,8 +802,22 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) { connector.addr_len), SyscallSucceeds()); + // Do two separate sends to ensure two segments are received. This is + // required for netstack where read is incorrectly assuming a whole + // segment is read when endpoint.Read() is called which is technically + // incorrect as the syscall that invoked endpoint.Read() may only + // consume it partially. This results in a case where a close() of + // such a socket does not trigger a RST in netstack due to the + // endpoint assuming that the endpoint has no unread data. EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), SyscallSucceedsWithValue(sizeof(i))); + + // TODO(gvisor.dev/issue/1449): Remove this block once netstack correctly + // generates a RST. + if (IsRunningOnGvisor()) { + EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0), + SyscallSucceedsWithValue(sizeof(i))); + } } }); -- cgit v1.2.3 From bf6429b944aed6de073c62ceb446cfaed5042dbc Mon Sep 17 00:00:00 2001 From: Brad Burlage Date: Fri, 10 Jan 2020 16:34:59 -0800 Subject: Don't set RWF_HIPRI on InvalidOffset test. This test fails on ubuntu 18.04 because preadv2 for some reason returns EOPNOTSUPP instead of EINVAL. Instead of root-causing the failure, I'm dropping the flag in the preadv2 call since it isn't under test in this scenario. PiperOrigin-RevId: 289188358 --- test/syscalls/linux/preadv2.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/syscalls/linux') diff --git a/test/syscalls/linux/preadv2.cc b/test/syscalls/linux/preadv2.cc index c9246367d..cd936ea90 100644 --- a/test/syscalls/linux/preadv2.cc +++ b/test/syscalls/linux/preadv2.cc @@ -202,7 +202,7 @@ TEST(Preadv2Test, TestInvalidOffset) { iov[0].iov_len = 0; EXPECT_THAT(preadv2(fd.get(), iov.get(), /*iovcnt=*/1, /*offset=*/-8, - /*flags=*/RWF_HIPRI), + /*flags=*/0), SyscallFailsWithErrno(EINVAL)); } -- cgit v1.2.3 From f54b9c0ee6e02f9c8bf32aa268c9028ff741bf7c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Mon, 13 Jan 2020 10:14:30 -0800 Subject: tests: fix errors detected by asan. PiperOrigin-RevId: 289467083 --- test/syscalls/linux/inotify.cc | 4 ++-- test/syscalls/linux/poll.cc | 3 ++- test/syscalls/linux/readv_common.cc | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'test/syscalls/linux') diff --git a/test/syscalls/linux/inotify.cc b/test/syscalls/linux/inotify.cc index 59ec9940a..fdef646eb 100644 --- a/test/syscalls/linux/inotify.cc +++ b/test/syscalls/linux/inotify.cc @@ -977,7 +977,7 @@ TEST(Inotify, WatchOnRelativePath) { ASSERT_NO_ERRNO_AND_VALUE(Open(file1.path(), O_RDONLY)); // Change working directory to root. - const char* old_working_dir = get_current_dir_name(); + const FileDescriptor cwd = ASSERT_NO_ERRNO_AND_VALUE(Open(".", O_PATH)); EXPECT_THAT(chdir(root.path().c_str()), SyscallSucceeds()); // Add a watch on file1 with a relative path. @@ -997,7 +997,7 @@ TEST(Inotify, WatchOnRelativePath) { // continue to hold a reference, random save/restore tests can fail if a save // is triggered after "root" is unlinked; we can't save deleted fs objects // with active references. - EXPECT_THAT(chdir(old_working_dir), SyscallSucceeds()); + EXPECT_THAT(fchdir(cwd.get()), SyscallSucceeds()); } TEST(Inotify, ZeroLengthReadWriteDoesNotGenerateEvent) { diff --git a/test/syscalls/linux/poll.cc b/test/syscalls/linux/poll.cc index 9e5aa7fd0..c42472474 100644 --- a/test/syscalls/linux/poll.cc +++ b/test/syscalls/linux/poll.cc @@ -275,7 +275,8 @@ TEST_F(PollTest, Nfds) { // Each entry in the 'fds' array refers to the eventfd and polls for // "writable" events (events=POLLOUT). This essentially guarantees that the // poll() is a no-op and allows negative testing of the 'nfds' parameter. - std::vector fds(max_fds, {.fd = efd.get(), .events = POLLOUT}); + std::vector fds(max_fds + 1, + {.fd = efd.get(), .events = POLLOUT}); // Verify that 'nfds' up to RLIMIT_NOFILE are allowed. EXPECT_THAT(RetryEINTR(poll)(fds.data(), 1, 1), SyscallSucceedsWithValue(1)); diff --git a/test/syscalls/linux/readv_common.cc b/test/syscalls/linux/readv_common.cc index 491d5f40f..2694dc64f 100644 --- a/test/syscalls/linux/readv_common.cc +++ b/test/syscalls/linux/readv_common.cc @@ -154,7 +154,7 @@ void ReadBuffersOverlapping(int fd) { char* expected_ptr = expected.data(); memcpy(expected_ptr, &kReadvTestData[overlap_bytes], overlap_bytes); memcpy(&expected_ptr[overlap_bytes], &kReadvTestData[overlap_bytes], - kReadvTestDataSize); + kReadvTestDataSize - overlap_bytes); struct iovec iovs[2]; iovs[0].iov_base = buffer.data(); -- cgit v1.2.3