Skip to content

ipcache: fix possible data race in kvstore synchronizer #39369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2025

Conversation

giorio94
Copy link
Member

@giorio94 giorio94 commented May 6, 2025

The blamed commit refactored the logic that synchronizes endpoint information into the kvstore to reduce the usage of globals. Yet, it still retrieved the kvstore client from the global accessor, pending its conversion to a proper cell. However, this turned out being affected by a data race, given that proper synchronization was not in place. Let's get this fixed by switching it to an atomic variable for the moment.

Fixes: f7e3eaa ("ipcache: inject kvstore synchronizer via Hive")
Fixes: #39294

The blamed commit refactored the logic that synchronizes endpoint
information into the kvstore to reduce the usage of globals. Yet,
it still retrieved the kvstore client from the global accessor,
pending its conversion to a proper cell. However, this turned out
being affected by a data race, given that proper synchronization
was not in place. Let's get this fixed by switching it to an atomic
variable for the moment.

Fixes: f7e3eaa ("ipcache: inject kvstore synchronizer via Hive")
Fixes: cilium#39294
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 requested a review from a team as a code owner May 6, 2025 15:55
@giorio94 giorio94 added the release-note/misc This PR makes changes that have no direct user impact. label May 6, 2025
@giorio94 giorio94 requested a review from bimmlerd May 6, 2025 15:55
@giorio94 giorio94 added the area/kvstore Impacts the KVStore package interactions. label May 6, 2025
@giorio94 giorio94 requested a review from aanm May 6, 2025 15:55
@giorio94
Copy link
Member Author

giorio94 commented May 6, 2025

/test

@giorio94 giorio94 enabled auto-merge May 6, 2025 16:23
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

whoof, nice catch.

@giorio94 giorio94 added this pull request to the merge queue May 7, 2025
Merged via the queue into cilium:main with commit 9ae0988 May 7, 2025
69 checks passed
@giorio94 giorio94 deleted the mio/ipcache-kvstore-data-race branch May 7, 2025 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kvstore Impacts the KVStore package interactions. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DATA RACE]: github.com/cilium/cilium/pkg/ipcache.(*IPIdentitySynchronizer).Upsert()
2 participants