From 24c33de748425e918033267313a4414b8ceb9727 Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Tue, 27 Oct 2020 18:11:46 -0700 Subject: Wake up any waiters on an ICMP error on UDP socket. This change wakes up any waiters when we receive an ICMP port unreachable control packet on an UDP socket as well as sets waiter.EventErr in the result returned by Readiness() when e.lastError is not nil. The latter is required where an epoll()/poll() is done after the error is already handled since we will never notify again in such cases. PiperOrigin-RevId: 339370469 --- pkg/tcpip/transport/udp/endpoint.go | 16 ++++++++++++---- test/syscalls/linux/udp_socket.cc | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/pkg/tcpip/transport/udp/endpoint.go b/pkg/tcpip/transport/udp/endpoint.go index d31177eb7..cdb5127ab 100644 --- a/pkg/tcpip/transport/udp/endpoint.go +++ b/pkg/tcpip/transport/udp/endpoint.go @@ -1369,6 +1369,12 @@ func (e *endpoint) Readiness(mask waiter.EventMask) waiter.EventMask { e.rcvMu.Unlock() } + e.lastErrorMu.Lock() + hasError := e.lastError != nil + e.lastErrorMu.Unlock() + if hasError { + result |= waiter.EventErr + } return result } @@ -1468,14 +1474,16 @@ func (e *endpoint) HandlePacket(r *stack.Route, id stack.TransportEndpointID, pk func (e *endpoint) HandleControlPacket(id stack.TransportEndpointID, typ stack.ControlType, extra uint32, pkt *stack.PacketBuffer) { if typ == stack.ControlPortUnreachable { e.mu.RLock() - defer e.mu.RUnlock() - if e.state == StateConnected { e.lastErrorMu.Lock() - defer e.lastErrorMu.Unlock() - e.lastError = tcpip.ErrConnectionRefused + e.lastErrorMu.Unlock() + e.mu.RUnlock() + + e.waiterQueue.Notify(waiter.EventErr) + return } + e.mu.RUnlock() } } diff --git a/test/syscalls/linux/udp_socket.cc b/test/syscalls/linux/udp_socket.cc index 6a488fec6..bc5bd9218 100644 --- a/test/syscalls/linux/udp_socket.cc +++ b/test/syscalls/linux/udp_socket.cc @@ -679,6 +679,43 @@ TEST_P(UdpSocketTest, SendToAddressOtherThanConnected) { SyscallSucceedsWithValue(sizeof(buf))); } +TEST_P(UdpSocketTest, ConnectAndSendNoReceiver) { + ASSERT_NO_ERRNO(BindLoopback()); + // Close the socket to release the port so that we get an ICMP error. + ASSERT_THAT(close(bind_.release()), SyscallSucceeds()); + + // Connect to loopback:bind_addr_ which should *hopefully* not be bound by an + // UDP socket. There is no easy way to ensure that the UDP port is not bound + // by another conncurrently running test. *This is potentially flaky*. + ASSERT_THAT(connect(sock_.get(), bind_addr_, addrlen_), SyscallSucceeds()); + + char buf[512]; + EXPECT_THAT(send(sock_.get(), buf, sizeof(buf), 0), + SyscallSucceedsWithValue(sizeof(buf))); + + constexpr int kTimeout = 1000; + // Poll to make sure we get the ICMP error back before issuing more writes. + struct pollfd pfd = {sock_.get(), POLLERR, 0}; + ASSERT_THAT(RetryEINTR(poll)(&pfd, 1, kTimeout), SyscallSucceedsWithValue(1)); + + // Next write should fail with ECONNREFUSED due to the ICMP error generated in + // response to the previous write. + ASSERT_THAT(send(sock_.get(), buf, sizeof(buf), 0), + SyscallFailsWithErrno(ECONNREFUSED)); + + // The next write should succeed again since the last write call would have + // retrieved and cleared the socket error. + ASSERT_THAT(send(sock_.get(), buf, sizeof(buf), 0), SyscallSucceeds()); + + // Poll to make sure we get the ICMP error back before issuing more writes. + ASSERT_THAT(RetryEINTR(poll)(&pfd, 1, kTimeout), SyscallSucceedsWithValue(1)); + + // Next write should fail with ECONNREFUSED due to the ICMP error generated in + // response to the previous write. + ASSERT_THAT(send(sock_.get(), buf, sizeof(buf), 0), + SyscallFailsWithErrno(ECONNREFUSED)); +} + TEST_P(UdpSocketTest, ZerolengthWriteAllowed) { // TODO(gvisor.dev/issue/1202): Hostinet does not support zero length writes. SKIP_IF(IsRunningWithHostinet()); -- cgit v1.2.3