From 79e2364933bbd34dceb0b9025e4e7d145fdf4492 Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Mon, 30 Nov 2020 20:11:33 -0800 Subject: Fix deadlock in UDP handleControlPacket path. Fixing the sendto deadlock exposed yet another deadlock where a lock inversion occurs on the handleControlPacket path where e.mu and demuxer.epsByNIC.mu are acquired in reverse order from say when RegisterTransportEndpoint is called in endpoint.Connect(). This fix sidesteps the issue by just making endpoint.state an atomic and gets rid of the need to acquire e.mu in e.HandleControlPacket. PiperOrigin-RevId: 344939895 --- test/syscalls/linux/udp_socket.cc | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) (limited to 'test/syscalls/linux') diff --git a/test/syscalls/linux/udp_socket.cc b/test/syscalls/linux/udp_socket.cc index 34255bfb8..90ef8bf21 100644 --- a/test/syscalls/linux/udp_socket.cc +++ b/test/syscalls/linux/udp_socket.cc @@ -375,8 +375,6 @@ TEST_P(UdpSocketTest, BindInUse) { } TEST_P(UdpSocketTest, ConnectWriteToInvalidPort) { - ASSERT_NO_ERRNO(BindLoopback()); - // Discover a free unused port by creating a new UDP socket, binding it // recording the just bound port and closing it. This is not guaranteed as it // can still race with other port UDP sockets trying to bind a port at the @@ -410,6 +408,35 @@ TEST_P(UdpSocketTest, ConnectWriteToInvalidPort) { ASSERT_EQ(optlen, sizeof(err)); } +TEST_P(UdpSocketTest, ConnectSimultaneousWriteToInvalidPort) { + // Discover a free unused port by creating a new UDP socket, binding it + // recording the just bound port and closing it. This is not guaranteed as it + // can still race with other port UDP sockets trying to bind a port at the + // same time. + struct sockaddr_storage addr_storage = InetLoopbackAddr(); + socklen_t addrlen = sizeof(addr_storage); + struct sockaddr* addr = reinterpret_cast(&addr_storage); + FileDescriptor s = + ASSERT_NO_ERRNO_AND_VALUE(Socket(GetFamily(), SOCK_DGRAM, IPPROTO_UDP)); + ASSERT_THAT(bind(s.get(), addr, addrlen), SyscallSucceeds()); + ASSERT_THAT(getsockname(s.get(), addr, &addrlen), SyscallSucceeds()); + EXPECT_EQ(addrlen, addrlen_); + EXPECT_NE(*Port(&addr_storage), 0); + ASSERT_THAT(close(s.release()), SyscallSucceeds()); + + // Now connect to the port that we just released. + ScopedThread t([&] { + ASSERT_THAT(connect(sock_.get(), addr, addrlen_), SyscallSucceeds()); + }); + + char buf[512]; + RandomizeBuffer(buf, sizeof(buf)); + // Send from sock_ to an unbound port. + ASSERT_THAT(sendto(sock_.get(), buf, sizeof(buf), 0, addr, addrlen_), + SyscallSucceedsWithValue(sizeof(buf))); + t.Join(); +} + TEST_P(UdpSocketTest, ReceiveAfterConnect) { ASSERT_NO_ERRNO(BindLoopback()); ASSERT_THAT(connect(sock_.get(), bind_addr_, addrlen_), SyscallSucceeds()); -- cgit v1.2.3