[Date Prev][Date Next] [Chronological] [Thread] [Top]

(ITS#8435) Uninitialized field slap_callback.sc_writewait



Full_Name: Hallvard B Furuseth
Version: 2.4.40 and later
OS: 
URL: 
Submission from: (NULL) (81.191.45.31)
Submitted by: hallvard


OpenLDAP 2.4.40 (2014-09-20) introduced added field sc_writewait
to slap_callback.  It is not always initialized.  That breaks
sending LDAP responses in slapd/result.c.  Typically a crash.

Third-party modules written before 2.4.40 break if they use
slap_callback variables without initializers (instead assigning
to each field), or if the initializer sets sc_private = non-NULL.
It gets put in sc_writewait instead, leaving sc_private=NULL.

(Initializers in C90-compatible code like OpenLDAP normally set
sc_private=NULL: sc_private is rarely a compile-time constant.)


Fixes:

If we move sc_writewait behind sc_private, that at least fixes
non-NULL sc_private initializers in third-party modules.  But
instead that can break such modules written after 2.4.40.

And we end up with 3 different ways to initialize slap_callback:
Before 2.4.40, after, and after 2.4.45.  I have not found a way
to code it so gcc compilation will fail if it is done wrong.

Draft branch "writewait" in my repo initializes the cases I found:
Syncrepl, backover, slapi, some overlays.  I did not check if this
is the correct fix.  ITS#8428 fixed another case.  However...

For the sake of third-party modules, I suppose the best fix in
OpenLDAP 2.4 is to remove slap_callback.sc_writewait if that is
feasible.  Create a separate variable field at the end of some
struct, which back-mdb callbacks should maintain and which
result.c calls.  Somewhere which has an existing bitflag variable
which can get a "the writewait field is valid" bit.  But I
haven't tried to figure out the details of that either.

Whatever we do, this time document the mess in the release notes
of this and some future releases.  And add a few macros to decide
which way to initialize.

I suggest to document that code which does not set sc_writewait,
should normally use an initializer with at most 4 fields where
sc_private is NULL, and set sc_private "by hand" later if it
should not be NULL.  Which is one reason not to use my branch,
let OpenLDAP show what code should look like instead.