From 40fe0ad0d9d0b8dd022d825e48a3d096b452bc71 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Tue, 11 Jun 2024 15:01:03 -0400 Subject: [PATCH] [v15] Fix panic in app dialing (#42786) * Fix panic in app dialing https://github.com/gravitational/teleport/pull/35501 incorrectly checked the length on the local servers variable instead of `t.c.servers` which could lead to panics like the one below. ```bash panic: runtime error: slice bounds out of range [1:0] goroutine 6558252 [running]: github.com/gravitational/teleport/lib/web/app.(*transport).DialContext(0xc00296d5f0, {0xae8bbd8, 0xc002596a20}, {0x42c525?, 0xc001dc4d80?}, {0x120?, 0x118?}) github.com/gravitational/teleport/lib/web/app/transport.go:264 +0x5dc net/http.(*Transport).dial(0xc002596a20?, {0xae8bbd8?, 0xc002596a20?}, {0x92ff0a2?, 0x4e4fc13?}, {0xc000b8dfc0?, 0xc00226f000?}) net/http/transport.go:1183 +0xd2 net/http.(*Transport).dialConn(0xc002f9cb40, {0xae8bbd8, 0xc002596a20}, {{}, 0x0, {0x9301a85, 0x5}, {0xc000b8dfc0, 0x1a}, 0x0}) net/http/transport.go:1625 +0x7e8 net/http.(*Transport).dialConnFor(0xae8bc10?, 0xc003340370) net/http/transport.go:1467 +0x9f created by net/http.(*Transport).queueForDial in goroutine 6562349 net/http/transport.go:1436 +0x3cb ``` * prevent modifying servers if changed --- lib/web/app/transport.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/web/app/transport.go b/lib/web/app/transport.go index 003001485f01f..4fa4b1d4828a6 100644 --- a/lib/web/app/transport.go +++ b/lib/web/app/transport.go @@ -281,12 +281,18 @@ func (t *transport) DialContext(ctx context.Context, _, _ string) (conn net.Conn break } - // eliminate any servers from the head of the list that were unreachable t.mu.Lock() - if i < len(servers) { - t.c.servers = t.c.servers[i:] - } else { - t.c.servers = nil + // Only attempt to tidy up the list of servers if they weren't altered + // while the dialing happened. Since the lock is only held initially when + // making the servers copy and released during the dials, another dial attempt + // may have already happened and modified the list of servers. + if len(servers) == len(t.c.servers) { + // eliminate any servers from the head of the list that were unreachable + if i < len(t.c.servers) { + t.c.servers = t.c.servers[i:] + } else { + t.c.servers = nil + } } t.mu.Unlock()