Skip to content

Commit a0085d9

Browse files
nybidarigvisor-bot
authored andcommitted
Close the endpoints when the routes cannot be found/restored during restore.
Log a warning and close the endpoints if the routes or NICs are no longer valid or cannot be found during restore. PiperOrigin-RevId: 792369060
1 parent 3bf460d commit a0085d9

File tree

5 files changed

+64
-11
lines changed

5 files changed

+64
-11
lines changed

pkg/tcpip/transport/icmp/endpoint_state.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020
"time"
2121

22+
"gvisor.dev/gvisor/pkg/log"
2223
"gvisor.dev/gvisor/pkg/tcpip"
2324
"gvisor.dev/gvisor/pkg/tcpip/stack"
2425
"gvisor.dev/gvisor/pkg/tcpip/transport"
@@ -51,9 +52,13 @@ func (e *endpoint) beforeSave() {
5152

5253
// Restore implements tcpip.RestoredEndpoint.Restore.
5354
func (e *endpoint) Restore(s *stack.Stack) {
54-
e.thaw()
55+
if err := e.net.Resume(s); err != nil {
56+
log.Warningf("Closing the ICMP endpoint as it cannot be restored, err: %v", err)
57+
e.Close()
58+
return
59+
}
5560

56-
e.net.Resume(s)
61+
e.thaw()
5762
if e.stack.IsSaveRestoreEnabled() {
5863
e.ops.InitHandler(e, e.stack, tcpip.GetStackSendBufferLimits, tcpip.GetStackReceiveBufferLimits)
5964
return
@@ -62,6 +67,9 @@ func (e *endpoint) Restore(s *stack.Stack) {
6267
e.stack = s
6368
e.ops.InitHandler(e, e.stack, tcpip.GetStackSendBufferLimits, tcpip.GetStackReceiveBufferLimits)
6469

70+
e.mu.Lock()
71+
defer e.mu.Unlock()
72+
6573
switch state := e.net.State(); state {
6674
case transport.DatagramEndpointStateInitial, transport.DatagramEndpointStateClosed:
6775
case transport.DatagramEndpointStateBound, transport.DatagramEndpointStateConnected:

pkg/tcpip/transport/internal/network/endpoint_state.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,14 @@ import (
2323
)
2424

2525
// Resume implements tcpip.ResumableEndpoint.Resume.
26-
func (e *Endpoint) Resume(s *stack.Stack) {
26+
func (e *Endpoint) Resume(s *stack.Stack) error {
2727
e.mu.Lock()
2828
defer e.mu.Unlock()
2929

3030
e.stack = s
31-
3231
for m := range e.multicastMemberships {
3332
if err := e.stack.JoinGroup(e.netProto, m.nicID, m.multicastAddr); err != nil {
34-
panic(fmt.Sprintf("e.stack.JoinGroup(%d, %d, %s): %s", e.netProto, m.nicID, m.multicastAddr, err))
33+
return fmt.Errorf("e.stack.JoinGroup(%d, %d, %s): %s", e.netProto, m.nicID, m.multicastAddr, err)
3534
}
3635
}
3736

@@ -42,17 +41,22 @@ func (e *Endpoint) Resume(s *stack.Stack) {
4241
case transport.DatagramEndpointStateBound:
4342
if info.ID.LocalAddress.BitLen() != 0 && !e.isBroadcastOrMulticast(info.RegisterNICID, e.effectiveNetProto, info.ID.LocalAddress) {
4443
if e.stack.CheckLocalAddress(info.RegisterNICID, e.effectiveNetProto, info.ID.LocalAddress) == 0 {
45-
panic(fmt.Sprintf("got e.stack.CheckLocalAddress(%d, %d, %s) = 0, want != 0", info.RegisterNICID, e.effectiveNetProto, info.ID.LocalAddress))
44+
return fmt.Errorf("got e.stack.CheckLocalAddress(%d, %d, %s) = 0, want != 0", info.RegisterNICID, e.effectiveNetProto, info.ID.LocalAddress)
4645
}
4746
}
4847
case transport.DatagramEndpointStateConnected:
4948
var err tcpip.Error
5049
multicastLoop := e.ops.GetMulticastLoop()
50+
// Release the connectedRoute if present.
51+
if e.connectedRoute != nil {
52+
e.connectedRoute.Release()
53+
}
5154
e.connectedRoute, err = e.stack.FindRoute(info.RegisterNICID, info.ID.LocalAddress, info.ID.RemoteAddress, e.effectiveNetProto, multicastLoop)
5255
if err != nil {
53-
panic(fmt.Sprintf("e.stack.FindRoute(%d, %s, %s, %d, %t): %s", info.RegisterNICID, info.ID.LocalAddress, info.ID.RemoteAddress, e.effectiveNetProto, multicastLoop, err))
56+
return fmt.Errorf("e.stack.FindRoute(%d, %s, %s, %d, %t): %s", info.RegisterNICID, info.ID.LocalAddress, info.ID.RemoteAddress, e.effectiveNetProto, multicastLoop, err)
5457
}
5558
default:
5659
panic(fmt.Sprintf("unhandled state = %s", state))
5760
}
61+
return nil
5862
}

pkg/tcpip/transport/internal/network/endpoint_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ func TestEndpointStateTransitions(t *testing.T) {
7676
remoteAddr tcpip.Address
7777
expectedRemoteAddr tcpip.Address
7878
checker func(*testing.T, *buffer.View)
79+
resume bool
7980
}{
8081
{
8182
name: "IPv4",
@@ -88,6 +89,7 @@ func TestEndpointStateTransitions(t *testing.T) {
8889
remoteAddr: ipv4RemoteAddr,
8990
expectedRemoteAddr: ipv4RemoteAddr,
9091
checker: v4Checker,
92+
resume: false,
9193
},
9294
{
9395
name: "IPv6",
@@ -100,6 +102,7 @@ func TestEndpointStateTransitions(t *testing.T) {
100102
remoteAddr: ipv6RemoteAddr,
101103
expectedRemoteAddr: ipv6RemoteAddr,
102104
checker: v6Checker,
105+
resume: false,
103106
},
104107
{
105108
name: "IPv4-mapped-IPv6",
@@ -112,6 +115,20 @@ func TestEndpointStateTransitions(t *testing.T) {
112115
remoteAddr: testutil.MustParse6("::ffff:0607:0809"),
113116
expectedRemoteAddr: ipv4RemoteAddr,
114117
checker: v4Checker,
118+
resume: false,
119+
},
120+
{
121+
name: "IPv4-resume",
122+
netProto: ipv4.ProtocolNumber,
123+
expectedMaxHeaderLength: header.IPv4MaximumHeaderSize,
124+
expectedNetProto: ipv4.ProtocolNumber,
125+
expectedLocalAddr: ipv4NICAddr,
126+
bindAddr: header.IPv4AllSystems,
127+
expectedBoundAddr: header.IPv4AllSystems,
128+
remoteAddr: ipv4RemoteAddr,
129+
expectedRemoteAddr: ipv4RemoteAddr,
130+
checker: v4Checker,
131+
resume: true,
115132
},
116133
}
117134

@@ -220,6 +237,19 @@ func TestEndpointStateTransitions(t *testing.T) {
220237
pkt.DecRef()
221238
}
222239

240+
if test.resume {
241+
// Remove all the routes so that Resume(..) fails.
242+
removed := s.RemoveRoutes(func(r tcpip.Route) bool {
243+
return r.NIC == nicID
244+
})
245+
if removed != 2 {
246+
t.Fatalf("wrong number of routes removed, got: %v, want: 2", removed)
247+
}
248+
if err := ep.Resume(s); err == nil {
249+
t.Fatalf("wrong error, got: nil, want: not nil")
250+
}
251+
}
252+
223253
ep.Close()
224254
if state := ep.State(); state != transport.DatagramEndpointStateClosed {
225255
t.Fatalf("got ep.State() = %s, want = %s", state, transport.DatagramEndpointStateClosed)

pkg/tcpip/transport/raw/endpoint_state.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"time"
2020

21+
"gvisor.dev/gvisor/pkg/log"
2122
"gvisor.dev/gvisor/pkg/tcpip"
2223
"gvisor.dev/gvisor/pkg/tcpip/stack"
2324
)
@@ -49,8 +50,12 @@ func (e *endpoint) beforeSave() {
4950

5051
// Restore implements tcpip.RestoredEndpoint.Restore.
5152
func (e *endpoint) Restore(s *stack.Stack) {
53+
if err := e.net.Resume(s); err != nil {
54+
log.Warningf("Closing the raw endpoint as it cannot be restored, err: %v", err)
55+
e.Close()
56+
return
57+
}
5258
e.setReceiveDisabled(false)
53-
e.net.Resume(s)
5459
if e.stack.IsSaveRestoreEnabled() {
5560
e.ops.InitHandler(e, e.stack, tcpip.GetStackSendBufferLimits, tcpip.GetStackReceiveBufferLimits)
5661
return

pkg/tcpip/transport/udp/endpoint_state.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"time"
2020

21+
"gvisor.dev/gvisor/pkg/log"
2122
"gvisor.dev/gvisor/pkg/tcpip"
2223
"gvisor.dev/gvisor/pkg/tcpip/stack"
2324
"gvisor.dev/gvisor/pkg/tcpip/transport"
@@ -50,12 +51,17 @@ func (e *endpoint) beforeSave() {
5051

5152
// Restore implements tcpip.RestoredEndpoint.Restore.
5253
func (e *endpoint) Restore(s *stack.Stack) {
53-
e.thaw()
54-
5554
e.mu.Lock()
5655
defer e.mu.Unlock()
5756

58-
e.net.Resume(s)
57+
if err := e.net.Resume(s); err != nil {
58+
log.Warningf("Closing the UDP endpoint as it cannot be restored, err: %v", err)
59+
e.closeLocked()
60+
return
61+
}
62+
63+
// Unfreeze the endpoint to handle packets.
64+
e.frozen = false
5965
if e.stack.IsSaveRestoreEnabled() {
6066
e.ops.InitHandler(e, e.stack, tcpip.GetStackSendBufferLimits, tcpip.GetStackReceiveBufferLimits)
6167
return

0 commit comments

Comments
 (0)