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

[Lachlan McIlroy]

Thanks Luc, here's the patch then:

Fix for the SIAL extension module to remove a call to sial_free() for an
uninitialised variable that can result in a segmentation violation when
unloading a sial script.

--- crash-6.0.2/extensions/sial.c	2011-12-23 02:17:31.000000000 +1100
+++ crash-6.0.2-fix/extensions/sial.c	2012-01-04 12:09:20.862910434 +1100
@@ -937,7 +937,6 @@
             }
             else rm_sial_cmd(name);
         }
-        sial_free(help_str);
     }
     free(help);
     return;


----- Original Message -----
> Yes - that problem was introduced (left behind) from the prior fix to
> unload. That is the right fix. Thanks Lachlan.
> 
> 
> -----Original Message-----
> From: crash-utility-bounces at redhat.com
> [mailto:crash-utility-bounces at redhat.com] On Behalf Of Lachlan
> McIlroy
> Sent: Tuesday, January 03, 2012 7:31 PM
> To: crash-utility at redhat.com
> Subject: [Crash-utility] freeing of uninitialised variable in
> reg_callback()
> 
> 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
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>