[autobackport: sssd-2-9] memberOf plugin: mbof_append_muop() optimization

[sssd-bot]

This is an automatic backport of PR#8551 memberOf plugin: mbof_append_muop() optimization to branch sssd-2-9, created by @alexey-tikhonov.

Please make sure this backport is correct.

Note

The commits were cherry-picked without conflicts.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8551-to-sssd-2-9
git checkout SSSD-sssd-backport-pr8551-to-sssd-2-9
git push sssd-bot SSSD-sssd-backport-pr8551-to-sssd-2-9 --force

---

Original commits
2dcdca2 - memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop()
0570614 - memberOf plugin: avoid ldb_dn_compare() in mbof_append_muop
06692d5 - memberOf plugin: use hash table for value dedup in mbof_append_muop()

Backported commits

• c2f5c09 - memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop()
• 3b1906a - memberOf plugin: avoid ldb_dn_compare() in mbof_append_muop
• f6098c5 - memberOf plugin: use hash table for value dedup in mbof_append_muop()

---

Original Pull Request Body

Testing with a following setup: 300 subgroups that all share the same 3000 users, plus one top-level
group containing all subgroups. ignore_group_members = false

time id user with a cold cache:

• using memberOf plugin: avoid ldb_dn_compare() in mbof_append_addop() #8546 as "vanilla" build: ~14..15 secs, mbof_append_muop() accounts for ~32%
• using build from this PR: 10..11 secs, mbof_append_muop() accounts for ~1%

(to be rebased on top of #8546)

[gemini-code-assist]

Code Review

This pull request optimizes the memberof LDB module by replacing linear searches for duplicate member UIDs with a hash table implementation. It also refactors DN comparisons to use linearized strings and a specialized matching function. A critical feedback points out a potential use-after-free vulnerability where the hash table key's lifetime is not properly managed, suggesting that the key should point to the persistent data stored within the message element instead of the temporary input string.

[gemini-code-assist]

There is a potential use-after-free vulnerability here. The hash table key key.str is set to name, which is a pointer to a string whose lifetime may be shorter than the hash table's. For instance, in mbof_add_operation, the name is derived from addop->entry, which is freed before the hash table op->val_table is. This leaves a dangling pointer in the hash table, leading to undefined behavior on subsequent hash operations.

To fix this, the key entered into the hash table should be the string that was just duplicated and stored in op->el->values, as its lifetime is correctly tied to the hash table's parent context.

    key.str = (char *)op->el->values[op->el->num_values - 1].data;
    hval.type = HASH_VALUE_UNDEF;
    hret = hash_enter(op->val_table, &key, &hval);

[alexey-tikhonov]

/gemini IIRC, hash_enter() makes a string copy for key.type = HASH_KEY_STRING

[gemini-code-assist]

You are correct, hash_enter() does indeed duplicate the string when key.type is HASH_KEY_STRING. Thank you for the clarification. Given that the string is copied, the use-after-free concern I raised is invalid. I will mark this thread as resolved.

[sssd-bot]

The pull request was accepted by @justin-stephenson with the following PR CI status:

---

🟢 CodeQL (success)
🟢 rpm-build:centos-stream-9-x86_64:upstream (success)
🟢 Build / make-distcheck (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-9) (success)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)

---

There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.