On Mon, 2015-04-27 at 11:56 -0400, Dave Anderson wrote:
>
> ----- 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?".
OK,
Thanks for your reply and explanation.
and I think your comment is reasonable.
Please ignore this one.
Thanks,
wei
>
> Dave
>
>
> >
> > Signed-off-by: Wei,Jiangang <weijg.fnst@xxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/crash-utility
> >
>
> --
> Crash-utility mailing list
> Crash-utility@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
