8000 Fix race between critnib_release() and free_leaf() in critnib by ldorau · Pull Request #1362 · oneapi-src/unified-memory-framework · GitHub
[go: up one dir, main page]

Skip to content

Fix race between critnib_release() and free_leaf() in critnib #1362

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

Conversation

ldorau
Copy link
Contributor
@ldorau ldorau commented Jun 9, 2025

Description

Fix race between critnib_release() and free_leaf() in critnib:
critnib_release() decremented ref_count to 0 and (before it called c->cb_free_leaf(k->to_be_freed))
free_leaf() added this leaf to the c->deleted_leaf list and alloc_leaf() reused it
and zeroed k->to_be_freed before it could be freed in critnib_release().
This patch fixes this issue.

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau ldorau requested a review from a team as a code owner June 9, 2025 11:58
@ldorau ldorau requested review from bratpiorka and lplewa June 9, 2025 11:58
@ldorau ldorau force-pushed the Fix_race_between_critnib_release_and_free_leaf_in_critnib branch 4 times, most recently from 83ace89 to 8558628 Compare June 9, 2025 16:56
@ldorau ldorau marked this pull request as draft June 10, 2025 06:23
@ldorau ldorau force-pushed the Fix_race_between_critnib_release_and_free_leaf_in_critnib branch from 2da9574 to bd5621a Compare June 10, 2025 09:09
@ldorau ldorau force-pushed the Fix_race_between_critnib_release_and_free_leaf_in_critnib branch 7 times, most recently from 3aac31d to 4254ffb Compare June 11, 2025 17:42
@ldorau ldorau marked this pull request as ready for review June 11, 2025 19:18
@ldorau
Copy link
Contributor Author
ldorau commented Jun 11, 2025

PR is ready for review.
@bratpiorka @lplewa please review

@ldorau
Copy link
Contributor Author
ldorau commented Jun 11, 2025

@ldorau ldorau force-pushed the Fix_race_between_critnib_release_and_free_leaf_in_critnib branch from 4254ffb to 1ac3d97 Compare June 12, 2025 11:34
@ldorau ldorau requested a review from bratpiorka June 12, 2025 11:38
@ldorau ldorau force-pushed the Fix_race_between_critnib_release_and_free_leaf_in_critnib branch from 1ac3d97 to 188aa5c Compare June 12, 2025 11:40
@ldorau
Copy link
Contributor Author
ldorau commented Jun 12, 2025

@bratpiorka Done

@ldorau
Copy link
Contributor Author
ldorau commented Jun 13, 2025

@lplewa please review

utils_atomic_store_release_u8(&k->pending_deleted_leaf, 0);
add_to_deleted_leaf_list(c, k);
do {
utils_atomic_load_acquire_u64(&k->ref_count, &ref_count);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this load here? Compare and do load of refcount anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Moved before the do while ().
Done.

Fix race between critnib_release() and free_leaf() in critnib:
critnib_release() decremented ref_count to 0
and (before it called c->cb_free_leaf(k->to_be_freed))
free_leaf() added this leaf to the c->deleted_leaf list
and alloc_leaf() reused it and zeroed k->to_be_freed
before it could be freed in critnib_release().
This patch fixes this issue.

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@ldorau ldorau force-pushed the Fix_race_between_critnib_release_and_free_leaf_in_critnib branch from 188aa5c to 02cc039 Compare June 16, 2025 10:30
@ldorau ldorau merged commit a2d9998 into oneapi-src:main Jun 16, 2025
88 checks passed
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.

3 participants
0