[Crash-utility] [PATCH 2/2] Fix for memory leaks

[Dave Anderson]

----- Original Message -----
> Without this patch, the storage reserved with the call
> to strdup won't be freed.

Did you even test this patch?  It causes this:

 $ patch -p1 < /tmp/leaks.patch
 patching file configure.c
 Hunk #1 succeeded at 768 (offset 64 lines).
 Hunk #2 succeeded at 1723 (offset 75 lines).

 $ make
 *** glibc detected *** ./configure: munmap_chunk(): invalid pointer: 0x0000000000404e50 ***
 ======= Backtrace: =========
 /lib64/libc.so.6[0x3e43e7ae16]
 ./configure[0x401e77]
 ./configure[0x400edd]
 /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e43e21735]
 ./configure[0x400d89]
 ======= Memory map: ========
 00400000-00407000 r-xp 00000000 fd:01 167898                             /var/CVS/crash-7.1.0/configure
 00607000-00608000 rw-p 00007000 fd:01 167898                             /var/CVS/crash-7.1.0/configure
 0184e000-0186f000 rw-p 00000000 00:00 0                                  [heap]
 3e43a00000-3e43a20000 r-xp 00000000 fd:01 1836136                        /usr/lib64/ld-2.15.so
 3e43c1f000-3e43c20000 r--p 0001f000 fd:01 1836136                        /usr/lib64/ld-2.15.so
 3e43c20000-3e43c21000 rw-p 00020000 fd:01 1836136                        /usr/lib64/ld-2.15.so
 3e43c21000-3e43c22000 rw-p 00000000 00:00 0 
 3e43e00000-3e43fac000 r-xp 00000000 fd:01 1836137                        /usr/lib64/libc-2.15.so
 3e43fac000-3e441ac000 ---p 001ac000 fd:01 1836137                        /usr/lib64/libc-2.15.so
 3e441ac000-3e441b0000 r--p 001ac000 fd:01 1836137                        /usr/lib64/libc-2.15.so
 3e441b0000-3e441b2000 rw-p 001b0000 fd:01 1836137                        /usr/lib64/libc-2.15.so
 3e441b2000-3e441b7000 rw-p 00000000 00:00 0 
 3e46a00000-3e46a15000 r-xp 00000000 fd:01 1853385                        /usr/lib64/libgcc_s-4.7.2-20120921.so.1
 3e46a15000-3e46c14000 ---p 00015000 fd:01 1853385                        /usr/lib64/libgcc_s-4.7.2-20120921.so.1
 3e46c14000-3e46c15000 rw-p 00014000 fd:01 1853385                        /usr/lib64/libgcc_s-4.7.2-20120921.so.1
 7ff116e8b000-7ff116e8e000 rw-p 00000000 00:00 0 
 7ff116ea7000-7ff116eab000 rw-p 00000000 00:00 0 
 7fff84a52000-7fff84a73000 rw-p 00000000 00:00 0                          [stack]
 7fff84b0c000-7fff84b0e000 r-xp 00000000 00:00 0                          [vdso]
 ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
 make: *** [all] Aborted (core dumped)
 $

It crashes because gdb_conf_flags is initialized to one of several possible
built-in strings before it is passed to get_extra_flags(): 

 void
 build_configure(struct supported_gdb_version *sp)
 {
         FILE *fp1, *fp2;
         char buf[512];
         char *target;
         char *target_CFLAGS;
         char *gdb_conf_flags;
         char *ldflags;
         char *cflags;

         get_current_configuration(sp);

         target = target_CFLAGS = NULL;

         gdb_conf_flags = GDB_TARGET_DEFAULT;

 ... [ cut ] ...
         or it may be set to one of these: 

			gdb_conf_flags = GDB_TARGET_X86_ON_X86_64;
			gdb_conf_flags = GDB_TARGET_PPC_ON_PPC64;
			gdb_conf_flags = GDB_TARGET_PPC64_ON_X86_64;
			gdb_conf_flags = GDB_TARGET_ARM_ON_X86;
			gdb_conf_flags = GDB_TARGET_ARM_ON_X86_64;
			gdb_conf_flags = GDB_TARGET_ARM64_ON_X86_64;
			gdb_conf_flags = GDB_TARGET_MIPS_ON_X86;
			gdb_conf_flags = GDB_TARGET_MIPS_ON_X86_64;
 ... [ cut ] ...

         gdb_conf_flags = get_extra_flags("GDBFLAGS.extra", gdb_conf_flags); 


And in the normal case where "GDBFLAGS.extra" does not exist, the hardwired string is 
passed back, and then the crash occurs when your patch tries to free it:

 char *
 get_extra_flags(char *filename, char *initial)
 {
         FILE *fp;
         char inbuf[512];
         char buf[512];

         if (!file_exists(filename))
                 return (initial ? initial : NULL);
 ...

Since configure.c simply runs and exits immediately when "make" is entered, it's hard to
qualify these as serious memory leaks.  They fall more under the category of "who cares?".  

Dave


> 
> Signed-off-by: Wei,Jiangang <weijg.fnst at cn.fujitsu.com>
> ---
>  configure.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/configure.c b/configure.c
> index 77ac81d..f78ad61 100755
> --- a/configure.c
> +++ b/configure.c
> @@ -704,6 +704,10 @@ build_configure(struct supported_gdb_version *sp)
>  
>  	}
>  
> +	free(ldflags);
> +	free(cflags);
> +	free(gdb_conf_flags);
> +
>  	makefile_create(&fp1, &fp2);
>  	show_configuration();
>  	make_build_data(&target[strlen("TARGET=")]);
> @@ -1644,6 +1648,9 @@ add_extra_lib(char *option)
>  			add_lsnappy++;
>  	}
>  
> +	free(ldflags);
> +	free(cflags);
> +
>  	if ((lzo || snappy) &&
>  	    file_exists("diskdump.o") && (unlink("diskdump.o") < 0)) {
>  		perror("diskdump.o");
> --
> 1.9.3
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>