[Date Prev][Date Next]
Re: (ITS#5340) REP_ENTRY_MODIFIABLE bug in dynlist
Pierangelo Masarati writes:
> Hallvard B Furuseth wrote:
>> But I don't see how the be_release() code can work now. It sounds like
>> be->be_release() functions must check (how?) that the entry was created
>> by 'be', and otherwise pass it on to the next overlay/backend or
>> otherwise to entry_free(). Might involve mucking with op->o_bd and
>> sr_entry->e_private, I suppose. Except maybe I'm missing some existing
>> magic since slapd doesn't regularly crash...
> Yes, but that's trivial: e_private must be NULL for temporary entries,
> and copying the entry loses it (no one is supposed to muck with it
> expect the entry's creator).
> And the appropriate o_bd of a
> (non-modifiable) entry can be easily computed from the entry's DN.
Hm? Can only the "outermost" backend/overlay create non-modifiable
I think any overlay can replace a non-modifiable rs->sr_entry value, as
long as it restores it on the way out (unless mustbe<freed/released>).
>>> Similarly, the existence of REP_ENTRY_MUSTBEFREED is not totally clear:
>>> in principle as soon as REP_ENTRY_MODIFYABLE is set, it should imply
>>> REP_ENTRY_MUSTBEFREED; the only difference in the semantics of the two
>> That's not my impression. (...)
Some functions create entries on the stack an send them MODIFIABLE - but
obviously without MUSTBEFREED. They use entry_clean() instead.
Others set MODIFIABLE but not MUSTBEFREED on non-stack entries, and call
entry_free after sending it. Or forget to free it - back-perl/search.c
if LDAP_SIZELIMIT_EXCEEDED. (Not patching yet - don't have time to test
it at the moment.)
> (...) A copy might be created by an overlay after receiving a
> read-only entry, but the same overlay might not actually perform the
> copy if it receives the entry from another overlay that already copied
> it, or from a proxy backend. However, after the entry is copied the
> overlay will have no means to determined who actually created the copy.
I don't think it matters who created a MUSTBEFREED copy:
> This might be an issue depending on the order cleanup handlers are
> called (didn't check what order they're called). My point is that
> temporary entries need to be freed at some point; who frees them should
> not be relevant...
Right, someone must call entry_free() and clear rs->sr_entry (so it's
not freed again), but entry_free() is not passed a backend parameter.
OTOH a MUSTBERELEASED entry must be released by the right
backend/overlay, and it would be strange for such an entry to be
MODIFIBALE. Though I guess it could be - if the backend has no cache,
and be_release just exists to clean up some data in e_private.
>> Others apparent problems (also not tested, I've just browsed the code):
>> Overlays that obey and reset MUSTBEFREED or MUSTRELEASE, do not
>> necessarily clear or set MODIFIABLE when setting a new entry.
> It seems to me that we should provide "smart" handlers to deal with
> preparing sr_entry for modification, and to take care of cleaning things
> up as appropriate. Those helpers should then be consistently used in
> the code.
Yup. And add some aggressive asserts - at least #ifdef LDAP_DEVEL.
Don't know when I'll have time for it though.