Re: (ITS#4148) fix for ITS 4134 reintroduces problem fixed in ITS 3980

[kevins@BMRB.Co.Uk]

Howard,

I think I've found and hopefully fixed the problem...

I found through observation and inserting a few printf's that there was
an issue in the section of ppolicy_modify starting at around line 1189,
where it iterates through each modification to see if it is a delete and
one of pwdGraceUseTime, pwdAccountLockedTime, pwdFailureTime.

It appeared that it was only spotting the first delete modification -
the ldif I was using had the following lines...

delete: pwdGraceUseTime
-
delete: pwdFailureTime

Neither attribute was present on the replica but it was dropping the
pwdGraceUseTime modification and not dropping the pwdFailureTime.

[I'm not really a C programmer, so some of the following diagnosis might
not be quite spot on!] 

I tracked this down to the for loop...

	for( prev = &op->oq_modify.rs_modlist, ml = *prev; ml;
	                        prev = &ml->sml_next, ml = *prev ) {

...and the drop itself...
   		  	  if ( drop ) {
                                *prev = ml->sml_next;
                                ml->sml_next = NULL;
                                slap_mods_free( ml, 1 );
                          }

What I think was happening was that if a modification was dropped
'ml->sml_next = NULL' in the drop code causes 'prev = &ml->sml_next, ml
= *prev' to result in ml being NULL and thus the loop exits without
considering further attributes.  I also suspect this is referencing
already freed memory [but I'm assuming what slap_mods_free() does]
sticking an 'ml = *prev' at the end of the drop code improves this but
still skips one attribute.

The following patch seems to solve the problem in my limited testing

--- servers/slapd/overlays/ppolicy.c.orig       Sat Nov  5 02:09:40 2005
+++ servers/slapd/overlays/ppolicy.c    Sat Nov  5 02:06:08 2005
@@ -1186,8 +1186,7 @@
                a_lock = attr_find( e->e_attrs,
ad_pwdAccountLockedTime );
                a_fail = attr_find( e->e_attrs, ad_pwdFailureTime );

-               for( prev = &op->oq_modify.rs_modlist, ml = *prev; ml;
-                       prev = &ml->sml_next, ml = *prev ) {
+               for( prev = &op->oq_modify.rs_modlist, ml = *prev; ml; )
{

                        if ( ml->sml_desc ==
slap_schema.si_ad_userPassword )
                                got_pw = 1;
@@ -1217,8 +1216,13 @@
                                        *prev = ml->sml_next;
                                        ml->sml_next = NULL;
                                        slap_mods_free( ml, 1 );
+                                       ml = *prev;
+                                       continue;
                                }
                        }
+
+                       prev = &ml->sml_next;
+                       ml = *prev;
                }

                /* If we're resetting the password, make sure grace,
accountlock,



Regards

Kevin
 


			

=================================================================

BMRB 
http://www.bmrb.co.uk
_________________________________________________________________
This message (and any attachment) is intended only for the 
recipient and may contain confidential and/or privileged 
material.  If you have received this in error, please contact the 
sender and delete this message immediately.  Disclosure, copying 
or other action taken in respect of this email or in 
reliance on it is prohibited.  BMRB Limited accepts no liability 
in relation to any personal emails, or content of any email which 
does not directly relate to our business.
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++