Issue 8435 - Uninitialized field slap_callback.sc_writewait
Summary: Uninitialized field slap_callback.sc_writewait
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-06 06:11 UTC by Hallvard Furuseth
Modified: 2017-06-01 22:09 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Hallvard Furuseth 2016-06-06 06:11:25 UTC
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.
Comment 1 Hallvard Furuseth 2016-06-12 08:52:51 UTC
changed notes
changed state Open to Test
moved from Incoming to Software Bugs
Comment 2 Quanah Gibson-Mount 2016-11-04 22:36:40 UTC
changed notes
changed state Test to Release
Comment 3 OpenLDAP project 2017-06-01 22:09:18 UTC
Fixed in master
Fixed in RE25
Fixed in RE24 (2.4.45)
Comment 4 Quanah Gibson-Mount 2017-06-01 22:09:18 UTC
changed notes
changed state Release to Closed