On Fri, Apr 25, 2008 at 11:36:40AM +0400, Alexander Bokovoy wrote:
> Jeremy,
>
> Jeremy Allison wrote:
>> @@ -67,6 +68,13 @@
>> #define CONST_DISCARD(type, ptr) ((type) ((void *) (ptr)))
>> +#ifndef SAFE_FREE
>> +#define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x=NULL;} } while(0)
>> +#endif
>> +
>> +#define MOUNT_PASSWD_SIZE 64
>> +#define DOMAIN_SIZE 64
>> +
>> const char *thisprogram;
>> int verboseflag = 0;
>> static int got_password = 0;
>> @@ -152,10 +160,7 @@ static void mount_cifs_usage(void)
>> printf("\nTo display the version number of the mount helper:");
>> printf("\n\t%s -V\n",thisprogram);
>> - if(mountpassword) {
>> - memset(mountpassword,0,64);
>> - free(mountpassword);
>> - }
>> + SAFE_FREE(mountpassword);

> Here we have non-equal behavior. Previously, the mountpassword content
> was zeroed before freeing it due to security reasons. As
> mount_cifs_usage() could be called multiple times (its call is in the
> getopt_long()'s loop) and, particulary, after password has been filled
> in, mountpassword's memory could still keep the password. Thus, memset()
> is still needed.


Ok, I'll replace the memset calls that aren't before exit().

There were a bunch of memset calls that were being done in an
exit() code path which were completely pointless, and I must have
gotten too agressive about removing them :-).

> There is still one place where (char *)calloc(65,1); is left after this
> commit: line 1197 (see below).


Thanks - I'll fix that.

Jeremy.