Re: (ITS#8404) Fix for an assertion failure when olcDbRewrite is modified on a meta or asyncmeta database

[hyc@symas.com]

nivanova@symas.com wrote:
> Full_Name: Nadezhda Ivanova
> Version: 2.5
> OS: Ubuntu 14.04
> URL: ftp://ftp.openldap.org/incoming/nadezhda-ivanova-160414.patch
> Submission from: (NULL) (78.83.54.234)
>
>
> The problem was reproduced when attempting to modify the dynamic configuration
> of meta and asyncmeta rewrite engine. On a meta database that has rewrite
> enabled and some rewrite rules configured, execute ldapmodify with the following
> LDIF:
> dn: olcMetaSub={0}uri,olcDatabase={1}meta,cn=config
> replace: olcDbRewrite
> olcDbRewrite: rewriteEngine "off"

Hm, multiple problems with this patch:

> diff --git a/servers/slapd/back-asyncmeta/config.c b/servers/slapd/back-asyncmeta/config.c
> index 1801a55..a4fbb91 100644
> --- a/servers/slapd/back-asyncmeta/config.c
> +++ b/servers/slapd/back-asyncmeta/config.c
> @@ -1797,14 +1797,66 @@ asyncmeta_back_cf_gen( ConfigArgs *c )
>
>  		case LDAP_BACK_CFG_SUFFIXM:	/* unused */
>  		case LDAP_BACK_CFG_REWRITE:
> -			if ( mt->mt_rwmap.rwm_bva_rewrite ) {
> -				ber_bvarray_free( mt->mt_rwmap.rwm_bva_rewrite );
> -				mt->mt_rwmap.rwm_bva_rewrite = NULL;
> -			}
> -			if ( mt->mt_rwmap.rwm_rw )
> +		{
> +			char			*argv0;
> +			if ( c->valx >= 0 ) {
> +				int i;
> +				
> +				for ( i = 0; !BER_BVISNULL( &mt->mt_rwmap.rwm_bva_rewrite[ i ] ); i++ )

Need a semicolon for the empty for-loop. Or, if it's intended that the 
following if-block is part of the loop, it needs to be properly enclosed in 
braces and indented.

> +				
> +				if ( c->valx >= i ) {
> +					rc = 1;
> +					break;
> +				}

> +				ber_memfree( mt->mt_rwmap.rwm_bva_rewrite[ c->valx ].bv_val );
> +				for ( i = c->valx; !BER_BVISNULL( &mt->mt_rwmap.rwm_bva_rewrite[ i + 1 ] ); i++ )
> +				{
> +					mt->mt_rwmap.rwm_bva_rewrite[ i ] = mt->mt_rwmap.rwm_bva_rewrite[ i + 1 ];
> +				}

> +				BER_BVZERO( &mt->mt_rwmap.rwm_bva_rewrite[ i ] );
> +
>  				rewrite_info_delete( &mt->mt_rwmap.rwm_rw );
> -			break;
> +				assert( mt->mt_rwmap.rwm_rw == NULL );
> +
> +				rc = asyncmeta_rwi_init( &mt->mt_rwmap.rwm_rw );
> +
> +				for ( i = 0; !BER_BVISNULL( &mt->mt_rwmap.rwm_bva_rewrite[ i ] ); i++ )
> +				{
> +					ConfigArgs ca = { 0 };
> +
> +					ca.line = mt->mt_rwmap.rwm_bva_rewrite[ i ].bv_val;
> +					ca.argc = 0;

Unnecessary initialization, you already set all of ca = {0}.

> +					init_config_argv( &ca );
> +					config_parse_ldif( &ca );
> +
> +					argv0 = ca.argv[ 0 ];

What is argv0 being saved for?

> +					if ( !strcasecmp( ca.argv[0], "suffixmassage" )) {
> +						rc = asyncmeta_suffixm_config( &ca, ca.argc, ca.argv, mt );
> +					} else {
> +						rc = rewrite_parse( mt->mt_rwmap.rwm_rw,
> +								    c->fname, c->lineno, ca.argc, ca.argv );
> +					}
> +				
> +					ca.argv[ 0 ] = argv0;

Why is argv0 being restored? All you're doing after this is freeing ca.argv 
anyway.
> +
> +					ch_free( ca.tline );
> +					ch_free( ca.argv );
>
> +					assert( rc == 0 );
> +				}
> +
> +			} else if ( mt->mt_rwmap.rwm_rw != NULL ) {
> +				if ( mt->mt_rwmap.rwm_bva_rewrite ) {
> +					ber_bvarray_free( mt->mt_rwmap.rwm_bva_rewrite );
> +					mt->mt_rwmap.rwm_bva_rewrite = NULL;
> +				}
> +				if ( mt->mt_rwmap.rwm_rw )
> +					rewrite_info_delete( &mt->mt_rwmap.rwm_rw );
> +				
> +				asyncmeta_rwi_init( &mt->mt_rwmap.rwm_rw );
> +			}		
> +		}
> +			break;
>  		case LDAP_BACK_CFG_MAP:
>  			if ( mt->mt_rwmap.rwm_bva_map ) {
>  				ber_bvarray_free( mt->mt_rwmap.rwm_bva_map );


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/