[Patches] patch: fix core dump in CCTK_ParameterSet() trying to free() an uninitialized pointer

[Jonathan Thornburg]

Hi,

If you
(a) set the same parameter twice, *and*
(b) the second setting is an invalid value,
then current-CVS Cactus core-dumps inside the flesh trying to  free()
a garbage (uninitialized) pointer.

For example, the following par file suffices to trigger the core-dump,
using only flesh parameters (*no* thorns activated):

   # this par file causes a core dump in current-CVS Cactus
   Cactus::cctk_timer_output = "off"
   Cactus::cctk_timer_output = "foo"	# an illegal value for this parameter

The core dump occurs in  src/main/Parameters.c  (CVS version 1.68) at
line 609, inside  CCTK_ParameterSet():
    599        /* check if a parameter is set more than once in a parfile */
    600        if (cctk_parameter_set_mask == PARAMETER_RECOVERY_PRE &&
    601            param->props->n_set > 0)
    602        {
    603          if (retval == 0)
    604          {
    605            new_value = CCTK_ParameterValString (param->props->name,
    606                                                 param->props->thorn);
    607            retval = strcmp (old_value, new_value) ? -10 : -11;
    608          }
    609          free (new_value);
    610        }
Notice that the  free()  at line 609 may be executed even if  new_value
was *not* assigned a value at line 605.  Alas,  new_value  is a local
variable in this function, and is not explicitly initialized, so it
has a garbage value initially... and line 605 is its only assignment
in this function.

The fix is easy -- just move the free() up to right after line 607,
so it's executed if and only if  new_value  has been assigned.

A patch is attached.  Tom, could you please look at this ASAP?
I think this is a fairly serious bug.....

ciao,

-- 
-- Jonathan Thornburg <jthorn at aei.mpg.de>
    Max-Planck-Institut fuer Gravitationsphysik (Albert-Einstein-Institut),
    Golm, Germany, "Old Europe"     http://www.aei.mpg.de/~jthorn/home.html
    "Washing one's hands of the conflict between the powerful and the
     powerless means to side with the powerful, not to be neutral."
                                       -- quote by Freire / poster by Oxfam
-------------- next part --------------
Index: ./src/main/Parameters.c
===================================================================
RCS file: /cactusdevcvs/Cactus/src/main/Parameters.c,v
retrieving revision 1.68
diff -u -r1.68 Parameters.c
--- ./src/main/Parameters.c	3 Feb 2006 10:50:34 -0000	1.68
+++ ./src/main/Parameters.c	21 Apr 2006 15:20:00 -0000
@@ -605,8 +605,8 @@
           new_value = CCTK_ParameterValString (param->props->name,
                                                param->props->thorn);
           retval = strcmp (old_value, new_value) ? -10 : -11;
+          free (new_value);
         }
-        free (new_value);
       }
 
       /* register another set operation */