Bug 222918

First of all, sorry about this late response.

> How reproducible:
> On my configuration always, however another server with different configuration
> (and database) survives.

Fixing the code not to access the contents of NULL is easy, but I'd like to
reproduce the problem in house.  Could you help me by sharing your env info? 
E.g., is it your custom schema?  Could you give us the definition of
cuniPrincipalName?  Also, have you indexed on the attribute?  If yes, what type
of indexes?  E.g., equality and presence?

Thank you for your support.

Created attachment 210881 [details]
cvs diffs

Files: servers/plugins/syntaxes/string.c servers/slapd/back-ldbm/index.c

Description: I tried to reproduce the problem, but it failed.
I tried the test with the stress with nsslapd-serial-lock: off, but the
condition was not the key.  My test always issues:
  ldap_modify: No such attribute
at the second deletion.
I tried index attribute as well as unindex, but no luck.

Anyway, although I could not reproduce the crash, but the stacktrace indicates
even if there is no attribute to delete, it calls index_addordel_values_sv with
NULL vals, which is not supposed to, I think.

Also, string_values2keys in string.c is not ready to accept NULL bvals.

I changed these two files so that even if the condition is realized, the server
won't crash.

In this code -
https://bugzilla.redhat.com/attachment.cgi?id=210881&action=diff#servers/plugins/syntaxes/string.c_sec1

if ( n == 0 ) {
    slapi_ch_free((void**)nbvals );
    return( 0 );
}

I think we need to set ivals (or *ivals) = NULL before we return.  It depends on
if the caller can handle ivals == NULL or expects ivals to be an array with the
first element NULL.

Also, I think you need to pass &nbvals to slapi_ch_free().

Otherwise, ok.

(In reply to comment #3)
> In this code -
>
https://bugzilla.redhat.com/attachment.cgi?id=210881&action=diff#servers/plugins/syntaxes/string.c_sec1
> 
> if ( n == 0 ) {
>     slapi_ch_free((void**)nbvals );
>     return( 0 );
> }
> 
> I think we need to set ivals (or *ivals) = NULL before we return.  It depends on
> if the caller can handle ivals == NULL or expects ivals to be an array with the
> first element NULL.
> 
> Also, I think you need to pass &nbvals to slapi_ch_free().
> 
> Otherwise, ok.

Thank you, Rich!  That was an important point...  The third arg is ivals.  If we
return NULL, then it'd make the server crash.  I'm making sure
string_values2keys not to return NULL ivals...
        if ( slapi_call_syntax_values2keys_sv( pi, (Slapi_Value**)va, &keyvals,
LDAP_FILTER_EQUALITY ) != 0 ) /* jcm cast */
        {
            [...]
            for ( i = 0; rc==LDAP_SUCCESS && va[i] != NULL; ++i )
            { 
                if ( keyvals[i] == NULL )  <== if keyvals is NULL, it crashes here!
                {

Created attachment 211091 [details]
revised diff for string.c

Based upon the comment from Rich, revised plugin/syntax/string.c:

-		 if ( n == 0 ) {
-			 slapi_ch_free((void**)ivals );
-			 return( 0 );
-		 }
+		 /* even if (n == 0), we should return the array nbvals w/ NULL

items */
		 *ivals = nbvals;
		 break;

Created attachment 211111 [details]
cvs commit message

Reivewed by Rich and Nathan (Thank you!!)

Checked in into CVS HEAD.

Created attachment 213441 [details]
schema definition

I attach the schema definition used by the server.

Created attachment 213451 [details]
server DSE

I attach the server configuration from DSE entry (keys and passwords are not
included :-)

Thanks a lot, Michal!  I'm using your data to verify the change.