Re: array-related segfault in bash-3

[William Park]

agriffis@gentoo.org wrote:
> Configuration Information [Automatically generated, do not change]:
> Machine: i686
> OS: linux-gnu
> Compiler: gcc
> Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='i686'
> -DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='i686-pc-linux-gnu'
> -DCONF_VENDOR='pc' -DLOCALEDIR='/usr/share/locale' -DPACKAGE='bash'
> -DSHELL -DHAVE_CONFIG_H  -I.  -I. -I./include -I./lib   -mcpu=athlon
> -O3 -pipe uname output: Linux time.flatmonk.org 2.6.6-rc3 #1 Thu May
> 13 09:43:41 EDT 2004 i686 AMD Athlon(tm) Processor AuthenticAMD
> GNU/Linux
> Machine Type: i686-pc-linux-gnu
> 
> Bash Version: 3.0
> Patch Level: 0
> Release Status: release
> 
> Description:
>         It is possible to segfault bash-3 by attempting to access a
>         local array in a function after it has been unset.
> 
> Repeat-By:
>         myfunc() { declare -a arr; unset arr; echo "${arr[*]}"; }
>         myfunc

Hmm... Relevant code seems to be makunbound() in variables.c:

    /* If we're unsetting a local variable and we're still executing inside
       the function, just mark the variable as invisible.  The function
       eventually called by pop_var_context() will clean it up later.  This
       must be done so that if the variable is subsequently assigned a new
       value inside the function, the `local' attribute is still present.
       We also need to add it back into the correct hash table. */
    if (old_var && local_p (old_var) && variable_context == old_var->context)
    {
+       /* Reset the attributes.  Preserve the export attribute if the variable
+          came from a temporary environment.  Make sure it stays local, and
+          make it invisible. */ 
+       old_var->attributes = (exported_p (old_var) && tempvar_p (old_var)) ? 
att_exported : 0;
+       VSETATTR (old_var, att_local);
        VSETATTR (old_var, att_invisible);
        FREE (value_cell (old_var));
        var_setvalue (old_var, (char *)NULL);
        INVALIDATE_EXPORTSTR (old_var);

where '+' marks new addition for Bash-3.0.  Essentially, it would go
something like

    1.  declare -a arr -- this makes 'arr' a local array variable.

    2.  unset arr -- this resets and sets the attribute to "local" and
        "invisible" only.  Since 'arr' is not exported, "exported" is
        not set.

    3.  ${arr[*]} -- this is valid array and subscript syntax.  So, it
        tries to find 'arr' variable, and it does, because 'arr' has not
        been deleted from the hash table.  But, it is not array
        variable, since "array" attribute is not set.  So, it thinks
        that 'arr' is regular shell variable, and tries to convert it to
        array, by putting the "old value" into new arr[0].
        
        However, the "old value" has been freed and zeroed.  When it
        tries to make a copy during the conversion, it calls
        add_string_to_list() which calls savestring() which is a wrapper
        around string(3) routines.  As you know, they don't like NULL.
        Hence, segfault.

        Bash-2.05b doesn't barf, because it only sets "invisible"
        attribute, keeping all other attributes.  So, 'arr' is
        considered to be array variable.  It doesn't segfault, even
        though it has NULL as content, because it calls
        array_to_word_list() which tests for NULL.
        

To Chet:

This is memory leak for array variable.  FREE() only frees the top level
ARRAY structure.  Array elements are still allocated in memory and will
become immortal, because pointer to them are lost.

-- 
William Park, Open Geometry Consulting, <opengeometry@yahoo.ca>
Toronto, Ontario, Canada