ipn/ipnlocal: fix taildrop deadlock

We update (*localNodeContext).AppendMatchingPeers's predicate to take a short-lived
PeerInfo struct instead of tailcfg.NodeView. This allows the predicate to use (*PeerInfo).HasCap(),
instead of calling back to localNodeContext causing the deadlock.

Fixes #15824

Signed-off-by: Nick Khyl <nickk@tailscale.com>
This commit is contained in:
Nick Khyl 2025-04-29 12:23:27 -05:00
parent 66371f392a
commit 6d8a414c06
No known key found for this signature in database
2 changed files with 48 additions and 6 deletions

View File

@ -1463,18 +1463,60 @@ func (b *LocalBackend) PeerCaps(src netip.Addr) tailcfg.PeerCapMap {
return b.currentNode().PeerCaps(src)
}
func (b *localNodeContext) AppendMatchingPeers(base []tailcfg.NodeView, pred func(tailcfg.NodeView) bool) []tailcfg.NodeView {
// PeerContext provides contextual information about a peer.
type PeerContext interface {
Node() tailcfg.NodeView
Caps() tailcfg.PeerCapMap
}
// peerContext is a short-lived struct containing information about a peer.
// It is not safe for concurrent use and [localNodeContext]'s mutex must be held
// when using it. Therefore, it must not be retained or used outside the scope
// of the predicate or callback it was passed to.
type peerContext struct {
self *localNodeContext
peer tailcfg.NodeView
}
func (p peerContext) Node() tailcfg.NodeView {
if p.self == nil {
panic("peerInfo used outside of its scope")
}
return p.peer
}
// HasCap reports whether the peer has the specified capability.
// It panics if used outside of the PeerInfo's scope.
func (p peerContext) Caps() tailcfg.PeerCapMap {
if p.self == nil {
panic("peerInfo used outside of its scope")
}
if p.peer.Addresses().Len() == 0 {
return nil
}
return p.self.peerCapsLocked(p.peer.Addresses().At(0).Addr())
}
// AppendMatchingPeers appends to base all peers in the netmap that match the predicate.
// The predicate is called with the node context's internal mutex locked,
// so it must not call back into the backend or the node context.
// The PeerInfo struct is only valid during the predicate call,
// and must not be retained or used outside of it.
func (b *localNodeContext) AppendMatchingPeers(base []tailcfg.NodeView, pred func(PeerContext) bool) []tailcfg.NodeView {
b.mu.Lock()
defer b.mu.Unlock()
ret := base
if b.netMap == nil {
return ret
}
pi := &peerContext{self: b}
for _, peer := range b.netMap.Peers {
if pred(peer) {
pi.peer = peer
if pred(pi) {
ret = append(ret, peer)
}
}
pi.self = nil
return ret
}

View File

@ -190,14 +190,14 @@ func (b *LocalBackend) FileTargets() ([]*apitype.FileTarget, error) {
if !b.capFileSharing {
return nil, errors.New("file sharing not enabled by Tailscale admin")
}
peers := cn.AppendMatchingPeers(nil, func(p tailcfg.NodeView) bool {
if !p.Valid() || p.Hostinfo().OS() == "tvOS" {
peers := cn.AppendMatchingPeers(nil, func(p PeerContext) bool {
if !p.Node().Valid() || p.Node().Hostinfo().OS() == "tvOS" {
return false
}
if self != p.User() {
if self != p.Node().User() {
return false
}
if p.Addresses().Len() != 0 && cn.PeerHasCap(p.Addresses().At(0).Addr(), tailcfg.PeerCapabilityFileSharingTarget) {
if p.Caps().HasCapability(tailcfg.PeerCapabilityFileSharingTarget) {
// Explicitly noted in the netmap ACL caps as a target.
return true
}