hubble: fix nil pointer dereference in Peer.Equal() when one Address is nil#45021
Merged
tklauser merged 1 commit intocilium:mainfrom Mar 31, 2026
Merged
Conversation
Contributor
|
/test |
devodev
approved these changes
Mar 27, 2026
Contributor
devodev
left a comment
There was a problem hiding this comment.
Thank you, this looks great!
…is nil When a peer has a nil Address field and is compared to a peer with a non-nil Address, Peer.Equal() panics with a nil pointer dereference. The original boolean expression short-circuits only when both addresses are nil; when just one is nil, it falls through to call .String() and .Network() on the nil net.Addr interface value. This causes hubble-relay to crash loop (exit code 2) in environments with dynamic node pools (e.g. GKE NAP, spot/preemptible nodes) where rapid PEER_ADDED notifications can produce Peer structs with nil Address fields during the upsert path in PeerManager.watchNotifications(). Replace the single boolean expression with a switch statement that explicitly handles three cases: both nil, one nil, and both non-nil. Add a test case covering the mixed nil/non-nil scenario. Signed-off-by: pesarkhobeee <ahmadian.farid.1988@gmail.com>
981c36a to
4429b3c
Compare
Author
|
Thanks @devodev for your attention and fast reply, I had to amend my commit to fix the Signed-off-by check failure |
Member
|
/test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Peer.Equal()inpkg/hubble/peer/types/peer.gopanics with a nil pointer dereference when one peer has a nilAddressand the other does not. This causes hubble-relay to crash loop (exit code 2) in environments with dynamic node pools.Root cause
The original expression:
When only one
Addressis nil, the first condition evaluates tofalse, so Go evaluates the second branch and calls.String()/.Network()on the nilnet.Addrinterface — panic.Fix
Replace the single boolean expression with a switch statement that explicitly handles three cases: both nil, one nil (mixed), and both non-nil.
Trigger scenario
In GKE with Node Auto-Provisioning (or any environment with node churn — autoscaling, spot/preemptible nodes), the hubble peer service fires multiple rapid
PEER_ADDEDnotifications for the same peer. When the peer address is unresolvable (e.g.connection refused),FromChangeNotification()returns aPeerwithAddress: nil. This nil-addressPeeris then passed toPeer.Equal()duringPeerManager.upsert()inwatchNotifications(), triggering the panic.Panic stack trace
Impact
Test plan
"one address nil other non-nil"toTestEqual— verifies no panic and correctfalseresultgo test ./pkg/hubble/peer/types/ -v