From: Luke Howard Date: Mon, 15 Sep 2003 06:53:54 +0000 (+0000) Subject: Modifications must be copied before calling slap_mods_check() because X-Git-Tag: OPENLDAP_REL_ENG_2_1_MP~724 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=b299d7e06757a3f1a2fda945dc3cdbc28460179b;p=openldap Modifications must be copied before calling slap_mods_check() because the pretty function may replace values which are owned by the SLAPI plugin slapi_entry_dup() optimization - avoid string re-encoding Fix slapi_add_internal() logic errors (was this ever tested?) Don't attempt to free entries that have been cached; see new internal slapi_add_entry_internal_locked() API --- diff --git a/servers/slapd/slapi/slapi_ops.c b/servers/slapd/slapi/slapi_ops.c index d7d7127bea..514f0e3389 100644 --- a/servers/slapd/slapi/slapi_ops.c +++ b/servers/slapd/slapi/slapi_ops.c @@ -19,6 +19,12 @@ */ #include "portable.h" + +#include +#include +#include +#include + #include #include #include @@ -32,8 +38,6 @@ static struct slap_listener slap_unknown_listener = { BER_BVC("UNKNOWN") }; -int bvptr2obj( struct berval **bvptr, struct berval **bvobj ); - static void internal_result_v3( Operation *op, @@ -258,9 +262,9 @@ static void slapiConnectionDestroy( Connection **pConn ) * the strings. */ static int -values2obj( +values2obj_copy( char **ppValue, - BerVarray *bvobj) + BerVarray *bvobj ) { int i; BerVarray tmpberval; @@ -278,8 +282,11 @@ values2obj( return LDAP_NO_MEMORY; } for ( i = 0; ppValue[i] != NULL; i++ ) { - tmpberval[i].bv_val = ppValue[i]; - tmpberval[i].bv_len = strlen( ppValue[i] ); + size_t len = strlen( ppValue[i] ); + + tmpberval[i].bv_val = slapi_ch_malloc( len + 1 ); + AC_MEMCPY( tmpberval[i].bv_val, ppValue[i], len + 1 ); + tmpberval[i].bv_len = len; } tmpberval[i].bv_val = NULL; tmpberval[i].bv_len = 0; @@ -289,23 +296,41 @@ values2obj( return LDAP_SUCCESS; } -static void -freeMods( Modifications *ml ) +static int +bvptr2obj_copy( + struct berval **bvptr, + BerVarray *bvobj ) { - /* - * Free a modification list whose values have been - * set with bvptr2obj() or values2obj() (ie. they - * do not own the pointer to the underlying values) - */ - Modifications *next; + int rc = LDAP_SUCCESS; + int i; + BerVarray tmpberval; - for ( ; ml != NULL; ml = next ) { - next = ml->sml_next; + if ( bvptr == NULL || *bvptr == NULL ) { + return LDAP_OTHER; + } - if ( ml->sml_bvalues ) slapi_ch_free( (void **)&ml->sml_bvalues ); - if ( ml->sml_nvalues ) slapi_ch_free( (void **)&ml->sml_nvalues ); - slapi_ch_free( (void **)&ml ); + for ( i = 0; bvptr != NULL && bvptr[i] != NULL; i++ ) { + ; /* EMPTY */ } + + tmpberval = (BerVarray)slapi_ch_malloc( (i + 1)*sizeof(struct berval)); + if ( tmpberval == NULL ) { + return LDAP_NO_MEMORY; + } + + for ( i = 0; bvptr[i] != NULL; i++ ) { + tmpberval[i].bv_val = slapi_ch_malloc( bvptr[i]->bv_len ); + tmpberval[i].bv_len = bvptr[i]->bv_len; + AC_MEMCPY( tmpberval[i].bv_val, bvptr[i]->bv_val, bvptr[i]->bv_len ); + } + tmpberval[i].bv_val = NULL; + tmpberval[i].bv_len = 0; + + if ( rc == LDAP_SUCCESS ) { + *bvobj = tmpberval; + } + + return rc; } /* @@ -338,7 +363,7 @@ LDAPModToEntry( op = (Operation *) slapi_ch_calloc(1, sizeof(Operation)); - if ( pEntry == NULL) { + if ( op == NULL) { rc = LDAP_NO_MEMORY; goto cleanup; } @@ -354,17 +379,25 @@ LDAPModToEntry( dn.bv_len = strlen(ldn); rc = dnPrettyNormal( NULL, &dn, &pEntry->e_name, &pEntry->e_nname, NULL ); - if ( rc != LDAP_SUCCESS ) + if ( rc != LDAP_SUCCESS ) { goto cleanup; + } if ( rc == LDAP_SUCCESS ) { - for ( i=0, pMod=mods[0]; rc == LDAP_SUCCESS && pMod != NULL; pMod=mods[++i]) { + for ( i = 0, pMod = mods[0]; rc == LDAP_SUCCESS && pMod != NULL; pMod = mods[++i]) { Modifications *mod; + if ( (pMod->mod_op & LDAP_MOD_BVALUES) != 0 ) { - /* attr values are in berval format */ - /* convert an array of pointers to bervals to an array of bervals */ - rc = bvptr2obj(pMod->mod_bvalues, &bv); - if (rc != LDAP_SUCCESS) goto cleanup; + /* + * Convert an array of pointers to bervals to + * an array of bervals. Note that we need to copy the + * values too, as the slap_mods_check() will free the + * original values after prettying; the modifications + * being passed in may not have been allocated on the + * heap. + */ + rc = bvptr2obj_copy( pMod->mod_bvalues, &bv ); + if ( rc != LDAP_SUCCESS ) goto cleanup; tmp.sml_type.bv_val = pMod->mod_type; tmp.sml_type.bv_len = strlen( pMod->mod_type ); tmp.sml_bvalues = bv; @@ -388,8 +421,8 @@ LDAPModToEntry( if ( pMod->mod_values == NULL ) { rc = LDAP_OTHER; } else { - rc = values2obj( pMod->mod_values, &bv ); - if (rc != LDAP_SUCCESS) goto cleanup; + rc = values2obj_copy( pMod->mod_values, &bv ); + if ( rc != LDAP_SUCCESS ) goto cleanup; tmp.sml_type.bv_val = pMod->mod_type; tmp.sml_type.bv_len = strlen( pMod->mod_type ); tmp.sml_bvalues = bv; @@ -436,12 +469,8 @@ LDAPModToEntry( } } - /* - * FIXME: slap_mods2entry is declared static - * in servers/slapd/add.c - */ rc = slap_mods2entry( modlist, &pEntry, repl_user, - 0, &text, textbuf, textlen ); + 0, &text, textbuf, textlen ); if (rc != LDAP_SUCCESS) { goto cleanup; } @@ -458,7 +487,7 @@ cleanup: if ( op ) slapi_ch_free( (void **)&op ); if ( modlist != NULL ) - freeMods( modlist ); + slap_mods_free( modlist ); if ( rc != LDAP_SUCCESS ) { if ( pEntry != NULL ) { slapi_entry_free( pEntry ); @@ -567,13 +596,13 @@ cleanup: #endif /* LDAP_SLAPI */ } -Slapi_PBlock * -slapi_add_entry_internal( - Slapi_Entry *e, +#ifdef LDAP_SLAPI +static Slapi_PBlock * +slapi_add_entry_internal_locked( + Slapi_Entry **e, LDAPControl **controls, int log_changes ) { -#ifdef LDAP_SLAPI Connection *pConn = NULL; Operation *op = NULL; Slapi_PBlock *pPB = NULL, *pSavePB = NULL; @@ -582,7 +611,7 @@ slapi_add_entry_internal( int isCritical; SlapReply rs = { REP_RESULT }; - if ( e == NULL ) { + if ( *e == NULL ) { rs.sr_err = LDAP_PARAM_ERROR; goto cleanup; } @@ -602,7 +631,7 @@ slapi_add_entry_internal( pPB = (Slapi_PBlock *)op->o_pb; op->o_ctrls = controls; - op->o_bd = select_backend( &e->e_nname, manageDsaIt, 0 ); + op->o_bd = select_backend( &((*e)->e_nname), manageDsaIt, 0 ); if ( op->o_bd == NULL ) { rs.sr_err = LDAP_PARTIAL_RESULTS; goto cleanup; @@ -610,15 +639,17 @@ slapi_add_entry_internal( op->o_dn = pConn->c_dn = op->o_bd->be_rootdn; op->o_ndn = pConn->c_ndn = op->o_bd->be_rootndn; - op->oq_add.rs_e = e; + op->oq_add.rs_e = *e; if ( op->o_bd->be_add ) { int repl_user = be_isupdate( op->o_bd, &op->o_ndn ); - if ( !op->o_bd->be_update_ndn.bv_len || repl_user ){ + if ( !op->o_bd->be_update_ndn.bv_len || repl_user ) { if ( (*op->o_bd->be_add)( op, &rs ) == 0 ) { if ( log_changes ) { replog( op ); } + be_entry_release_w( op, *e ); + *e = NULL; } } else { rs.sr_err = LDAP_REFERRAL; @@ -640,12 +671,34 @@ cleanup: slapiConnectionDestroy( &pConn ); return( pSavePB ); +} +#endif /* LDAP_SLAPI */ + +Slapi_PBlock * +slapi_add_entry_internal( + Slapi_Entry *e, + LDAPControl **controls, + int log_changes ) +{ +#ifdef LDAP_SLAPI + Slapi_PBlock *pb; + Slapi_Entry *entry; + + /* + * We make a copy to avoid an entry that may be freed later + * by the caller being placed in the cache. + */ + entry = slapi_entry_dup( e ); + pb = slapi_add_entry_internal_locked( &entry, controls, log_changes ); + if ( entry != NULL ) { + slapi_entry_free( entry ); + } + return pb; #else return NULL; -#endif /* LDAP_SLAPI */ +#endif } - Slapi_PBlock * slapi_add_internal( char *dn, @@ -673,7 +726,8 @@ slapi_add_internal( } if ( rc == LDAP_SUCCESS ) { - if((pEntry = LDAPModToEntry( dn, mods )) == NULL) { + pEntry = LDAPModToEntry( dn, mods ); + if ( pEntry == NULL ) { rc = LDAP_OTHER; } } @@ -682,10 +736,10 @@ slapi_add_internal( pb = slapi_pblock_new(); slapi_pblock_set( pb, SLAPI_PLUGIN_INTOP_RESULT, (void *)rc ); } else { - pb = slapi_add_entry_internal( pEntry, controls, log_changes ); + pb = slapi_add_entry_internal_locked( &pEntry, controls, log_changes ); } - if ( pEntry ) { + if ( pEntry != NULL ) { slapi_entry_free(pEntry); } @@ -909,7 +963,7 @@ slapi_modify_internal( * convert an array of pointers to bervals * to an array of bervals */ - rs.sr_err = bvptr2obj( pMod->mod_bvalues, &bv ); + rs.sr_err = bvptr2obj_copy( pMod->mod_bvalues, &bv ); if ( rs.sr_err != LDAP_SUCCESS ) goto cleanup; tmp.sml_type.bv_val = pMod->mod_type; @@ -926,7 +980,7 @@ slapi_modify_internal( mod->sml_bvalues = tmp.sml_bvalues; mod->sml_nvalues = tmp.sml_nvalues; } else { - rs.sr_err = values2obj( pMod->mod_values, &bv ); + rs.sr_err = values2obj_copy( pMod->mod_values, &bv ); if ( rs.sr_err != LDAP_SUCCESS ) goto cleanup; tmp.sml_type.bv_val = pMod->mod_type; @@ -1017,7 +1071,7 @@ cleanup: slapi_ch_free( (void **)&dn.bv_val ); if ( modlist != NULL ) - freeMods( modlist ); + slap_mods_free( modlist ); if ( pConn != NULL ) { pSavePB = pPB; diff --git a/servers/slapd/slapi/slapi_utils.c b/servers/slapd/slapi/slapi_utils.c index f1e43aab5d..1bc7311292 100644 --- a/servers/slapd/slapi/slapi_utils.c +++ b/servers/slapd/slapi/slapi_utils.c @@ -170,26 +170,17 @@ Slapi_Entry * slapi_entry_dup( Slapi_Entry *e ) { #ifdef LDAP_SLAPI - char *tmp = NULL; - Slapi_Entry *tmpEnt; - int len = 0; - - tmp = slapi_entry2str( e, &len ); - if ( tmp == NULL ) { - return (Slapi_Entry *)NULL; - } + Slapi_Entry *ret; - tmpEnt = (Slapi_Entry *)str2entry( tmp ); - if ( tmpEnt == NULL ) { - slapi_ch_free( (void **)&tmp ); - return (Slapi_Entry *)NULL; - } - - if (tmp != NULL) { - slapi_ch_free( (void **)&tmp ); - } + ret = (Slapi_Entry *)slapi_ch_calloc( 1, sizeof(*ret) ); - return tmpEnt; + ret->e_id = e->e_id; + ber_dupbv( &ret->e_name, &e->e_name ); + ber_dupbv( &ret->e_nname, &e->e_nname ); + ret->e_attrs = attrs_dup( e->e_attrs ); + ret->e_ocflags = e->e_ocflags; + ber_dupbv( &ret->e_bv, &e->e_bv ); + ret->e_private = NULL; #else /* LDAP_SLAPI */ return NULL; #endif /* LDAP_SLAPI */