Skip to content

ipa: fix memory leak in ipa_s2n_get_list iteration#8654

Draft
alexey-tikhonov wants to merge 1 commit intoSSSD:masterfrom
alexey-tikhonov:ipa-mem-leak
Draft

ipa: fix memory leak in ipa_s2n_get_list iteration#8654
alexey-tikhonov wants to merge 1 commit intoSSSD:masterfrom
alexey-tikhonov:ipa-mem-leak

Conversation

@alexey-tikhonov
Copy link
Copy Markdown
Member

Per-step temporaries (retoid, retdata, parsed names, encoded requests) were allocated on the long-lived state context and never freed between iterations. Use a per-step talloc context and free retoid/retdata immediately after consumption.

Implementation assisted-by: Claude Code (Opus 4.6)

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a per-step memory context (step_ctx) to the ipa_s2n_get_list_state structure and updates several function calls to use this context, facilitating better memory management during iterations. Additionally, it ensures that override_attrs, retoid, and retdata are explicitly freed. The review feedback identifies a potential memory accumulation issue where state->attrs should also be freed at the start of each step to cover execution paths that bypass the standard cleanup.

Comment on lines +1335 to +1337
talloc_zfree(state->step_ctx);
talloc_zfree(state->override_attrs);
state->step_ctx = talloc_new(state);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The state->attrs structure should also be freed at the beginning of each step. While it is currently freed in ipa_s2n_get_list_next, that function is only called during the Extended Operation (EXOP) path. If an iteration processes an IPA object (the ipa_id_get_account_info_send path), state->attrs from a previous EXOP iteration will remain in memory until the next EXOP iteration or until the entire request completes, leading to unnecessary memory accumulation.

    talloc_zfree(state->step_ctx);
    talloc_zfree(state->override_attrs);
    talloc_zfree(state->attrs);
    state->step_ctx = talloc_new(state);

Per-step temporaries (retoid, retdata, parsed names, encoded requests)
were allocated on the long-lived state context and never freed between
iterations. Use a per-step talloc context and free retoid/retdata
immediately after consumption.

Implementation assisted-by: Claude Code (Opus 4.6)
@alexey-tikhonov alexey-tikhonov added the coverity Trigger a coverity scan label Apr 29, 2026
@alexey-tikhonov
Copy link
Copy Markdown
Member Author

Note: Covscan is clean.

@alexey-tikhonov alexey-tikhonov added Waiting for review and removed coverity Trigger a coverity scan labels Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant