[autobackport: sssd-2-9] KRB5: fix mem leak in authenticate_stored_users()#8537
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request backports two memory leak fixes. The fix in src/providers/krb5/krb5_delayed_online_authentication.c is correct and properly frees a hash table on all exit paths. However, the changes in src/util/find_uid.c introduce a merge conflict that must be resolved. The code in its current state will not compile. I have provided a critical review comment with a suggested resolution for the conflict.
8b543a4 to
91735dd
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request addresses a couple of memory leaks by ensuring hash_table_t objects are properly destroyed in all code paths. The changes in src/providers/krb5/krb5_delayed_online_authentication.c add cleanup calls in both success and error scenarios. Similarly, the change in src/util/find_uid.c prevents a leak if get_active_uid_linux() fails. While the fixes are correct, I've suggested improvements to check the return value of hash_destroy() to ensure the cleanup is successful and to log any failures, which is crucial for robustness, especially on success paths.
alexey-tikhonov
added
backport-to-sssd-2-9-4
91735dd to
90d4ba7
Compare
Reviewed-by: Justin Stephenson <[email protected]> Reviewed-by: Sumit Bose <[email protected]> (cherry picked from commit 74e7bc6)
Reviewed-by: Justin Stephenson <[email protected]> Reviewed-by: Sumit Bose <[email protected]> (cherry picked from commit 5b85b64)
|
The pull request was accepted by @alexey-tikhonov with the following PR CI status: 🟢 CodeQL (success) There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging. |
90d4ba7 to
000aa50
Compare
4 participants
This is an automatic backport of PR#8517 KRB5: fix mem leak in
authenticate_stored_users()to branch sssd-2-9, created by @alexey-tikhonov.Caution
@alexey-tikhonov The patches did not apply cleanly. It is necessary to resolve conflicts before merging this pull request. Commits that introduced conflict are marked with
CONFLICT!.You can push changes to this pull request
Original commits
74e7bc6 - KRB5: fix mem leak in
authenticate_stored_users()5b85b64 - UTIL: fix mem leak if
get_active_uid()failsBackported commits
authenticate_stored_users()get_active_uid()failsConflicting Files Information (check for deleted and re-added files)
get_active_uid()failsOriginal Pull Request Body