[Date Prev][Date Next]
(ITS#8435) Uninitialized field slap_callback.sc_writewait
Full_Name: Hallvard B Furuseth
Version: 2.4.40 and later
Submission from: (NULL) (188.8.131.52)
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.)
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.