summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorBhasker Hariharan <bhaskerh@google.com>2020-10-27 18:11:46 -0700
committergVisor bot <gvisor-bot@google.com>2020-10-27 18:13:46 -0700
commit24c33de748425e918033267313a4414b8ceb9727 (patch)
treef429c4a4ecb260936e9af0b09bec9a0466b869fc
parent1c2836da37261c47cb8372e3ae5a49adab369694 (diff)
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
-rw-r--r--pkg/tcpip/transport/udp/endpoint.go16
-rw-r--r--test/syscalls/linux/udp_socket.cc37
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());