From 123173f839f2ff327f3f90f39e0b149f9d15f4e9 Mon Sep 17 00:00:00 2001 From: ignoramous Date: Fri, 4 Oct 2024 06:04:38 +0530 Subject: [PATCH 1/3] tcpip/udp: avoid deadlock in forwader.CreateEndpoint --- pkg/tcpip/transport/udp/forwarder.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/tcpip/transport/udp/forwarder.go b/pkg/tcpip/transport/udp/forwarder.go index 7950abe58..d702c54ad 100644 --- a/pkg/tcpip/transport/udp/forwarder.go +++ b/pkg/tcpip/transport/udp/forwarder.go @@ -76,15 +76,17 @@ func (r *ForwarderRequest) CreateEndpoint(queue *waiter.Queue) (tcpip.Endpoint, netHdr := r.pkt.Network() if err := ep.net.Bind(tcpip.FullAddress{NIC: r.pkt.NICID, Addr: netHdr.DestinationAddress(), Port: r.id.LocalPort}); err != nil { + ep.closeLocked() return nil, err } if err := ep.net.Connect(tcpip.FullAddress{NIC: r.pkt.NICID, Addr: netHdr.SourceAddress(), Port: r.id.RemotePort}); err != nil { + ep.closeLocked() return nil, err } if err := r.stack.RegisterTransportEndpoint([]tcpip.NetworkProtocolNumber{r.pkt.NetworkProtocolNumber}, ProtocolNumber, r.id, ep, ep.portFlags, tcpip.NICID(ep.ops.GetBindToDevice())); err != nil { - ep.Close() + ep.closeLocked() return nil, err } From 741bf52370b8eaf5ba5a948169e70176133fa23d Mon Sep 17 00:00:00 2001 From: ignoramous Date: Fri, 4 Oct 2024 06:14:17 +0530 Subject: [PATCH 2/3] tcpip/udp: document preconditions for endpoint.closeLocked() --- pkg/tcpip/transport/udp/endpoint.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/tcpip/transport/udp/endpoint.go b/pkg/tcpip/transport/udp/endpoint.go index 785ea47b2..7e78782f2 100644 --- a/pkg/tcpip/transport/udp/endpoint.go +++ b/pkg/tcpip/transport/udp/endpoint.go @@ -160,11 +160,16 @@ func (e *endpoint) Abort() { // associated with it. func (e *endpoint) Close() { e.mu.Lock() + e.closeLocked() + e.mu.Unlock() +} +// Preconditions: e.mu is locked. +// +checklocks:e.mu +func (e *endpoint) closeLocked() { switch state := e.net.State(); state { case transport.DatagramEndpointStateInitial: case transport.DatagramEndpointStateClosed: - e.mu.Unlock() return case transport.DatagramEndpointStateBound, transport.DatagramEndpointStateConnected: id := e.net.Info().ID @@ -201,7 +206,6 @@ func (e *endpoint) Close() { e.net.Shutdown() e.net.Close() e.readShutdown = true - e.mu.Unlock() e.waiterQueue.Notify(waiter.EventHUp | waiter.EventErr | waiter.ReadableEvents | waiter.WritableEvents) } From ab3c4c85a3eed4493fbe448c7c53ce4b9c03c21a Mon Sep 17 00:00:00 2001 From: ignoramous Date: Sat, 5 Oct 2024 00:14:04 +0530 Subject: [PATCH 3/3] tcpip/udp: defer mutex unlock --- pkg/tcpip/transport/udp/endpoint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tcpip/transport/udp/endpoint.go b/pkg/tcpip/transport/udp/endpoint.go index 7e78782f2..0bd7db350 100644 --- a/pkg/tcpip/transport/udp/endpoint.go +++ b/pkg/tcpip/transport/udp/endpoint.go @@ -160,8 +160,8 @@ func (e *endpoint) Abort() { // associated with it. func (e *endpoint) Close() { e.mu.Lock() + defer e.mu.Unlock() e.closeLocked() - e.mu.Unlock() } // Preconditions: e.mu is locked.