quicreuse: nil pointer panic on every QUIC dial when netroute.New() fails (e.g. Android)
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 6.9k
- Forks
- 1.3k
- Avg merge
- 13d 21h
- Merged PRs (30d)
- 1
Description
### What happens
`defaultSourceIPSelectorFn` returns a non-nil `SourceIPSelector` wrapping a nil
`routing.Router` when `netroute.New()` fails. Callers discard the error and then
guard on the wrapper, which is not nil — so the guard passes and the next dial
dereferences the nil router and takes the whole process down.
On Android this is not an edge case: an unprivileged app generally cannot read
the kernel route table over netlink, so `netroute.New()` fails routinely, and
**every** QUIC dial can crash. We saw 21 crashes in 19 hours on one device.
```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x703ba332b0]
goroutine 1195 [running]:
quicreuse.(*netrouteSourceIPSelector).PreferredSourceIPForDestination(...)
p2p/transport/quicreuse/reuse.go:485
quicreuse.(*reuse).transportWithAssociationForDial(...)
p2p/transport/quicreuse/reuse.go:278
quicreuse.(*ConnManager).TransportWithAssociationForDial(...)
p2p/transport/quicreuse/connmgr.go:341
quicreuse.(*ConnManager).DialQUIC(...)
p2p/transport/quicreuse/connmgr.go:315
quic.(*transport).dialWithScope(...)
p2p/transport/quic/transport.go:190
quic.(*transport).Dial(...)
p2p/transport/quic/transport.go:174
swarm.(*Swarm).dialAddr(...)
p2p/net/swarm/swarm_dial.go:606
swarm.(*dialLimiter).executeDial(...)
p2p/net/swarm/limiter.go:213
```
### Why the existing nil check does not catch it
```go
// connmgr.go
func defaultSourceIPSelectorFn() (SourceIPSelector, error) {
r, err := netroute.New()
return &netrouteSourceIPSelector{routes: r}, err // non-nil selector, nil router
}
// reuse.go — both call sites discard the error
r.routes, _ = r.sourceIPSelectorFn() // "Ignore the error, there's nothing we can do about it."
// reuse.go: transportWithAssociationForDial
if router != nil { // passes: the WRAPPER is not nil
src, err := router.PreferredSourceIPForDestination(raddr)
// reuse.go
func (s *netrouteSourceIPSelector) PreferredSourceIPForDestination(dst *net.UDPAddr) (net.IP, error) {
_, _, src, err := s.routes.Route(dst.IP) // s.routes is nil -> panic
return src, err
}
```
The comment at the discard sites ("nothing we can do about it") is reasonable
only if a failed construction yields something harmless. It does not: it yields
a value that passes the guard and panics on use.
### Reproduction
```go
func TestPreferredSourceIPWithNoRouterPanics(t *testing.T) {
s := &netrouteSourceIPSelector{routes: nil}
s.PreferredSourceIPForDestination(&net.UDPAddr{IP: net.IPv4(1, 1, 1, 1), Port: 443})
}
```
Panics with the same `addr=0x18` as the field crash. The same happens end to end
via `OverrideSourceIPSelector` returning `(&netrouteSourceIPSelector{routes: nil}, err)`
and then dialing.
### Suggested fix
Return an untyped nil when there is no router, so the existing guard means what
it says. A typed nil (`(*netrouteSourceIPSelector)(nil)`) would still satisfy
`!= nil` and change nothing.
```go
func newSourceIPSelector(r routing.Router, err error) (SourceIPSelector, error) {
if err != nil || r == nil {
return nil, err
}
return &netrouteSourceIPSelector{routes: r}, nil
}
func defaultSourceIPSelectorFn() (SourceIPSelector, error) {
return newSourceIPSelector(netroute.New())
}
```
Optionally also make the method refuse rather than dereference, so a caller
constructing the struct directly cannot reintroduce this:
```go
func (s *netrouteSourceIPSelector) PreferredSourceIPForDestination(dst *net.UDPAddr) (net.IP, error) {
if s == nil || s.routes == nil {
return nil, errors.New("quicreuse: no route table available")
}
...
}
```
Degraded behaviour is a dial without source-IP affinity, which is an
optimisation for multi-homed hosts rather than a correctness requirement — and
is already what happens on every platform where netroute fails, except that
today it panics instead of degrading.
Happy to open a PR if this looks right.
### Environment
- go-libp2p v0.41.1; the same code is on master as of 2026-09-08
- Android 16 (`BP4A.251205.006`), arm64, via gomobile
- Also affects iOS builds of the same core, though `netroute.New()` usually
succeeds there, so it does not fire in practice
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with defaultSourceIPSelectorFn in connmgr.go and the two selector call sites in reuse.go, then run TestPreferredSourceIPWithNoRouterPanics from the reproduction. Verify that a failed netroute.New() no longer causes a QUIC dial to dereference a nil router and that dialing degrades without source-IP affinity.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100