https://bugs.openldap.org/show_bug.cgi?id=10290
Issue ID: 10290 Summary: Combination of syncrepl+rwm+syncprov frees the wrong modlist Product: OpenLDAP Version: unspecified Hardware: All OS: All Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: overlays Assignee: bugs@openldap.org Reporter: ondra@mistotebe.net Target Milestone: ---
An MPR setup with rwm enabled (regardless of configuration it seems) will crash with the provided modlist being freed twice. This is the sequence of events of what is stored in op->orm_modlist, allocated and freed by whom, replacing the actual pointers to make it easier to track:
syncrepl_message_to_op: preparing a modify with 0xoriginal syncrepl_op_modify: old modlist 0xoriginal replacing with 0xsyncrepl_op_modify rwm_op_modify: old modlist 0xsyncrepl_op_modify replacing with 0xrwm_op_modify <modify happens> syncrepl_modify_cb: freeing 0xsyncrepl_op_modify, replacing with 0xoriginal (forgetting 0xrwm_op_modify) rwm_op_rollback: freeing 0xoriginal replacing with 0xsyncrepl_op_modify syncrepl_message_to_op: went in with 0xoriginal, got 0xsyncrepl_op_modify back syncrepl_message_to_op: freeing 0xsyncrepl_op_modify
Not sure who is at fault: syncrepl_modify_cb is the one freeing the wrong modlist, but then if backover were to work with an actual "stack", running response callbacks in the opposite order from the request, things would have been ok too.
https://bugs.openldap.org/show_bug.cgi?id=10290
--- Comment #1 from Ondřej Kuzník ondra@mistotebe.net --- Another note.
Even if code is changed for rwm/syncrepl try to cooperate despite the callback order being what it is, things go wrong anyway: rwm allocates its modlist with ch_malloc, syncrepl gets it from op->o_tmpmemctx. They cannot safely free each other's modlists, knowing nothing about what the right way to go about it would be.
https://bugs.openldap.org/show_bug.cgi?id=10290
--- Comment #2 from Howard Chu hyc@openldap.org --- (In reply to Ondřej Kuzník from comment #1)
Another note.
Even if code is changed for rwm/syncrepl try to cooperate despite the callback order being what it is, things go wrong anyway: rwm allocates its modlist with ch_malloc, syncrepl gets it from op->o_tmpmemctx. They cannot safely free each other's modlists, knowing nothing about what the right way to go about it would be.
ch_free() will correctly free either.
https://bugs.openldap.org/show_bug.cgi?id=10290
--- Comment #3 from Ondřej Kuzník ondra@mistotebe.net --- On Mon, Dec 09, 2024 at 02:34:43PM +0000, openldap-its@openldap.org wrote:
ch_free() will correctly free either.
On the contrary, objects allocated from inside op->o_tmpmemctx are not safe to free by anyone else (and ch_malloc()'d pointers can be freed by anyone so long as they share the same free() fallback). And slap_mods_free can't use it because it's not been passed enough information.
Luckily, syncrepl keeps two separate callbacks and there seems to be a way to maintain a stack-like ordering: overlay_callback_after_backover.
This is what MR!737 does here: https://git.openldap.org/openldap/openldap/-/merge_requests/737
https://bugs.openldap.org/show_bug.cgi?id=10290
--- Comment #4 from Howard Chu hyc@openldap.org --- (In reply to Ondřej Kuzník from comment #3)
On Mon, Dec 09, 2024 at 02:34:43PM +0000, openldap-its@openldap.org wrote:
ch_free() will correctly free either.
On the contrary, objects allocated from inside op->o_tmpmemctx are not safe to free by anyone else (and ch_malloc()'d pointers can be freed by anyone so long as they share the same free() fallback). And slap_mods_free can't use it because it's not been passed enough information.
NONSENSE.
https://git.openldap.org/openldap/openldap/-/blob/master/servers/slapd/ch_ma...
https://bugs.openldap.org/show_bug.cgi?id=10290
--- Comment #5 from Ondřej Kuzník ondra@mistotebe.net --- On Mon, Dec 09, 2024 at 06:41:00PM +0000, openldap-its@openldap.org wrote:
On Mon, Dec 09, 2024 at 02:34:43PM +0000, openldap-its@openldap.org wrote:
ch_free() will correctly free either.
On the contrary, objects allocated from inside op->o_tmpmemctx are not safe to free by anyone else (and ch_malloc()'d pointers can be freed by anyone so long as they share the same free() fallback). And slap_mods_free can't use it because it's not been passed enough information.
NONSENSE.
https://git.openldap.org/openldap/openldap/-/blob/master/servers/slapd/ch_ma...
Ok, that's not even *hinted at* anywhere else. And it took a while to trace down the mechanisms of whether the thread context might be set correctly when needed in this case because syncrepl manages its own Operations.
Regardless, syncrepl_op_modify created modifications cannot be safely passed to slap_mods_free (I tried that), it might be that the `#define free ch_free` in proto-slap.h somehow doesn't actually work, or down to something I did, I didn't investigate further. And even if it was all fine and something else I did was at fault, rwm still needs to call `slap_mods_free( x, 1 )` which definitely isn't compatible with the ones coming from syncrepl_op_modify.
https://bugs.openldap.org/show_bug.cgi?id=10290
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Target Milestone|--- |2.6.10 Assignee|bugs@openldap.org |ondra@mistotebe.net Keywords|needs_review | Status|UNCONFIRMED |IN_PROGRESS
--- Comment #6 from Quanah Gibson-Mount quanah@openldap.org --- https://git.openldap.org/openldap/openldap/-/merge_requests/737
-
openldap-its＠openldap.org