[Crash-utility] freeing of uninitialised variable in reg_callback()

[Lachlan McIlroy]

I'm using crash 6.0.2 and I'm regularly seeing this segfault from sial when unloading a sial script:

crash> extend ./sial.so
Core LINUX_RELEASE == '2.6.18-238.12.1.el5'
< Sial interpreter version 3.0 >
        Loading sial commands from /usr/share/sial/crash:/home/lmcilroy/.sial .... Done.
./sial.so: shared object loaded
crash> load script.sial 
crash> unload script.sial
*** glibc detected *** crash: double free or corruption (!prev): 0x00000000071999b0 ***
Segmentation fault

Program received signal SIGSEGV, Segmentation fault.
0x0000003b61c74f32 in malloc_consolidate () from /lib64/libc.so.6
(gdb) bt
#0  0x0000003b61c74f32 in malloc_consolidate () from /lib64/libc.so.6
#1  0x0000003b61c77bd2 in _int_malloc () from /lib64/libc.so.6
#2  0x0000003b61c78c88 in calloc () from /lib64/libc.so.6
#3  0x0000003b6180a98f in _dl_new_object () from /lib64/ld-linux-x86-64.so.2
#4  0x0000003b61805e4f in _dl_map_object_from_fd () from /lib64/ld-linux-x86-64.so.2
#5  0x0000003b61807bd2 in _dl_map_object () from /lib64/ld-linux-x86-64.so.2
#6  0x0000003b61812530 in dl_open_worker () from /lib64/ld-linux-x86-64.so.2
#7  0x0000003b6180dd76 in _dl_catch_error () from /lib64/ld-linux-x86-64.so.2
#8  0x0000003b61811fb7 in _dl_open () from /lib64/ld-linux-x86-64.so.2
#9  0x0000003b61d1afb0 in do_dlopen () from /lib64/libc.so.6
#10 0x0000003b6180dd76 in _dl_catch_error () from /lib64/ld-linux-x86-64.so.2
#11 0x0000003b61d1b107 in __libc_dlopen_mode () from /lib64/libc.so.6
#12 0x0000003b61cf3cc1 in backtrace () from /lib64/libc.so.6
#13 0x0000003b61c6f147 in __libc_message () from /lib64/libc.so.6
#14 0x0000003b61c74ac6 in malloc_printerr () from /lib64/libc.so.6
#15 0x00007f85babefe7a in sial_deletefile (name=0x462bf78 "script.sial") at sial_func.c:320
#16 0x00007f85babf5d36 in sial_loadunload (load=0, name=<value optimized out>, silent=0) at sial_api.c:1289
#17 0x00007f85babec77d in unload_cmd () at sial.c:775
#18 0x000000000045d4df in exec_command () at main.c:751
#19 0x000000000045d6ea in main_loop () at main.c:699
#20 0x0000000000557019 in captured_command_loop (data=<value optimized out>) at ./main.c:228
#21 0x00000000005552eb in catch_errors (func=<value optimized out>, func_args=<value optimized out>, errstring=<value optimized out>, mask=<value optimized out>) at exceptions.c:531
#22 0x0000000000556d26 in captured_main (data=<value optimized out>) at ./main.c:958
#23 0x00000000005552eb in catch_errors (func=<value optimized out>, func_args=<value optimized out>, errstring=<value optimized out>, mask=<value optimized out>) at exceptions.c:531
#24 0x0000000000555ee4 in gdb_main (args=0x98) at ./main.c:973
#25 0x0000000000555f1e in gdb_main_entry (argc=<value optimized out>, argv=<value optimized out>) at ./main.c:993
#26 0x000000000045e24f in main (argc=<value optimized out>, argv=<value optimized out>) at main.c:603

I've traced the fault to extensions/sial.c:reg_callback() where it is freeing 'help_str' without it being initialised first.

void
reg_callback(char *name, int load)
{
char fname[MAX_SYMNAMELEN+sizeof("_usage")+1];
char *help_str, *opt_str;
char **help=malloc(sizeof *help * 5);

    if(!help) return;
    snprintf(fname, sizeof(fname), "%s_help", name);
    if(sial_chkfname(fname, 0)) {
        snprintf(fname, sizeof(fname), "%s_usage", name);
        if(sial_chkfname(fname, 0)) {
            if(load) {
                opt_str=sial_strdup((char*)(unsigned long)sial_exefunc(fname, 0));
                snprintf(fname, sizeof(fname), "%s_help", name);
                help_str=sial_strdup((char*)(unsigned long)sial_exefunc(fname, 0));
                help[0]=sial_strdup(name);
                help[1]="";
                help[2]=sial_strdup(opt_str);
                help[3]=sial_strdup(help_str);
                help[4]=0;
                add_sial_cmd(name, run_callback, help, 0);
                sial_free(help_str);
                sial_free(opt_str);
                return;
            }
            else rm_sial_cmd(name);
        }
        sial_free(help_str);  <--- segfaults here.
    }
    free(help);
    return;
}

I don't see how 'help_str' should be initialised at this point and removing the 'sial_free(help_str)' prevents the problem - is that the right thing to do here?

Lachlan