Skip to content

fix(state): guard StateEventChans iteration with StateEventChansLock#2775

Open
Aamod007 wants to merge 1 commit into
kubearmor:mainfrom
Aamod007:fix/state-event-chans-race
Open

fix(state): guard StateEventChans iteration with StateEventChansLock#2775
Aamod007 wants to merge 1 commit into
kubearmor:mainfrom
Aamod007:fix/state-event-chans-race

Conversation

@Aamod007

@Aamod007 Aamod007 commented Jul 15, 2026

Copy link
Copy Markdown

Purpose of PR?:

Fixes #2774

PushContainerEvent, PushNodeEvent and PushNamespaceEvent in KubeArmor/state/data.go iterate sa.StateEventChans without holding StateEventChansLock, while addStateEventChan/removeStateEventChan mutate the map and close the channels under that lock on every WatchState client connect/disconnect. Since the runtime handlers push events from separate goroutines (go dm.StateAgent.PushContainerEvent(...)), a client disconnecting mid-broadcast crashes the daemon with fatal error: concurrent map iteration and map write or panic: send on closed channel.

This PR takes StateEventChansLock.RLock() around the broadcast loop in all three functions. The sends are already non-blocking (select/default), so holding the read lock during fan-out is cheap, and because removeStateEventChan closes channels under the write lock, the read lock also rules out the send-on-closed-channel panic.

Does this PR introduce a breaking change?

No.

If the changes in this PR are manually verified, list down the scenarios covered::

  • Added TestPushContainerEventRace which simulates WatchState clients connecting/disconnecting concurrently with container event pushes. On current main it fails under go test -race (data race between PushContainerEvent and removeStateEventChan) and also panics with send on closed channel. With this fix, go test -race -count=1 -run TestPushContainerEventRace ./state/ passes.
  • go vet ./state/ clean.

Additional information for reviewer? :

Failure output on main without the fix:

WARNING: DATA RACE
Read at ... by goroutine 11:
  ...state.(*StateAgent).PushContainerEvent()  KubeArmor/state/data.go:85
Previous write at ... by goroutine 10:
  runtime.closechan()
  ...state.(*StateAgent).removeStateEventChan() KubeArmor/state/stateAgent.go:92
...
panic: send on closed channel

Checklist:

PushContainerEvent, PushNodeEvent and PushNamespaceEvent iterate
sa.StateEventChans without holding StateEventChansLock, while
addStateEventChan/removeStateEventChan mutate the map and close the
channels under the lock on every WatchState client connect/disconnect.

Since the runtime handlers push events from separate goroutines
(go dm.StateAgent.PushContainerEvent(...)), a client disconnecting
mid-broadcast triggers either a concurrent map iteration/write fatal
error or a send on closed channel panic, crashing the whole daemon.

Take a read lock around the broadcast loops so channels cannot be
removed or closed while an event is being fanned out. Add a regression
test that fails under -race (and panics) without this fix.

Signed-off-by: Aamod007 <aamodkumar2006@gmail.com>
@Aamod007 Aamod007 force-pushed the fix/state-event-chans-race branch from 4fa3057 to c6b6370 Compare July 15, 2026 15:36
default:
}
sa.removeStateEventChan(uid)
_ = i

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here i is already used in the for loop, so the compiler doesn't consider it unused, making the assignment pointless. It neither suppresses a compiler error nor does anything at runtime. i know its pedantic but still worth pointing out dead code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

state agent: Push*Event iterate StateEventChans without StateEventChansLock — daemon crash (concurrent map write / send on closed channel)

2 participants