[binutils-gdb] SEGV in ppc64_elf_get_synthetic_symtab reading a separate debug file

[Jan Kratochvil <jkratoch at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=80c570537e380c1b8e48754c0ddbce2abcde2d00

commit 80c570537e380c1b8e48754c0ddbce2abcde2d00
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Thu Feb 26 14:08:01 2015 +0100

    SEGV in ppc64_elf_get_synthetic_symtab reading a separate debug file
    
    The attached patch fixes the SEGV and lets GDB successfully
    load all kernel modules installed by default on RHEL 7.
    
    Valgrind on F-21 x86_64 host has shown me more clear what is the problem:
    
    Reading symbols from /home/jkratoch/t/cordic.ko...Reading symbols from
    /home/jkratoch/t/cordic.ko.debug...=================================================================
    ==22763==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000461c8 at pc 0x150cdbd bp 0x7fffffffc7e0 sp 0x7fffffffc7d0
    READ of size 8 at 0x6120000461c8 thread T0
        #0 0x150cdbc in ppc64_elf_get_synthetic_symtab /home/jkratoch/redhat/gdb-test-asan/bfd/elf64-ppc.c:3282
        #1 0x8c5274 in elf_read_minimal_symbols /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1205
        #2 0x8c55e7 in elf_symfile_read /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1268
    [...]
    0x6120000461c8 is located 264 bytes inside of 288-byte region [0x6120000460c0,0x6120000461e0)
    freed by thread T0 here:
        #0 0x7ffff715454f in __interceptor_free (/lib64/libasan.so.1+0x5754f)
        #1 0xde9cde in xfree common/common-utils.c:98
        #2 0x9a04f7 in do_my_cleanups common/cleanups.c:155
        #3 0x9a05d3 in do_cleanups common/cleanups.c:177
        #4 0x8c538a in elf_read_minimal_symbols /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1229
        #5 0x8c55e7 in elf_symfile_read /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1268
    [...]
    previously allocated by thread T0 here:
        #0 0x7ffff71547c7 in malloc (/lib64/libasan.so.1+0x577c7)
        #1 0xde9b95 in xmalloc common/common-utils.c:41
        #2 0x8c4da2 in elf_read_minimal_symbols /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1147
        #3 0x8c55e7 in elf_symfile_read /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1268
    [...]
    SUMMARY: AddressSanitizer: heap-use-after-free /home/jkratoch/redhat/gdb-test-asan/bfd/elf64-ppc.c:3282 ppc64_elf_get_synthetic_symtab
    [...]
    ==22763==ABORTING
    
    A similar case a few lines later I have fixed in 2010 by:
            https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3f1eff0a2c7f0e7078f011f55b8e7f710aae0cc2
    
    My testcase does not always reproduce it but at least a bit:
     * GDB without ppc64 target (even as a secondary one) is reported as "untested"
     * ASAN-built GDB with ppc64 target always crashes (and PASSes with this fix)
     * unpatched non-ASAN-built GDB with ppc64 target crashes from commandline
     * unpatched non-ASAN-built GDB with ppc64 target PASSes from runtest (?)
    
    gdb/ChangeLog
    2015-02-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* elfread.c (elf_read_minimal_symbols): Use bfd_alloc for
    	bfd_canonicalize_symtab.
    
    gdb/testsuite/ChangeLog
    2015-02-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* gdb.arch/cordic.ko.bz2: New file.
    	* gdb.arch/cordic.ko.debug.bz2: New file.
    	* gdb.arch/ppc64-symtab-cordic.exp: New file.

Diff:
---
 gdb/ChangeLog                                  |   5 +++
 gdb/elfread.c                                  |   6 ++-
 gdb/testsuite/ChangeLog                        |   6 +++
 gdb/testsuite/gdb.arch/cordic.ko.bz2           | Bin 0 -> 2208 bytes
 gdb/testsuite/gdb.arch/cordic.ko.debug.bz2     | Bin 0 -> 910 bytes
 gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp |  51 +++++++++++++++++++++++++
 6 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9934346..2a2dadd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-02-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* elfread.c (elf_read_minimal_symbols): Use bfd_alloc for
+	bfd_canonicalize_symtab.
+
 2015-02-25  John Baldwin  <jhb@FreeBSD.org>
 
 	* amd64fbsd-nat.c: Include sys/user.h.
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 65c63f0..4a6576f 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1144,8 +1144,10 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
 
   if (storage_needed > 0)
     {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      make_cleanup (xfree, symbol_table);
+      /* Memory gets permanently referenced from ABFD after
+	 bfd_canonicalize_symtab so it must not get freed before ABFD gets.  */
+
+      symbol_table = bfd_alloc (abfd, storage_needed);
       symcount = bfd_canonicalize_symtab (objfile->obfd, symbol_table);
 
       if (symcount < 0)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c7b1c15..504e2f0 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-02-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.arch/cordic.ko.bz2: New file.
+	* gdb.arch/cordic.ko.debug.bz2: New file.
+	* gdb.arch/ppc64-symtab-cordic.exp: New file.
+
 2015-02-25  Yao Qi  <yao.qi@linaro.org>
 
 	* gdb.xml/tdesc-regs.exp: Set core-regs to aarch64-core.xml for
diff --git a/gdb/testsuite/gdb.arch/cordic.ko.bz2 b/gdb/testsuite/gdb.arch/cordic.ko.bz2
new file mode 100644
index 0000000..8cb5d66
Binary files /dev/null and b/gdb/testsuite/gdb.arch/cordic.ko.bz2 differ
diff --git a/gdb/testsuite/gdb.arch/cordic.ko.debug.bz2 b/gdb/testsuite/gdb.arch/cordic.ko.debug.bz2
new file mode 100644
index 0000000..8685f82
Binary files /dev/null and b/gdb/testsuite/gdb.arch/cordic.ko.debug.bz2 differ
diff --git a/gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp b/gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp
new file mode 100644
index 0000000..d9a3f13
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp
@@ -0,0 +1,51 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+set kobz2file ${srcdir}/${subdir}/cordic.ko.bz2
+set kofile ${objdir}/${subdir}/cordic.ko
+set kodebugbz2file ${srcdir}/${subdir}/cordic.ko.debug.bz2
+set kodebugfile ${objdir}/${subdir}/cordic.ko.debug
+
+if {[catch "system \"bzip2 -dc ${kobz2file} >${kofile}\""] != 0} {
+    untested "failed bzip2 for ${kobz2file}"
+    return -1
+}
+if {[catch "system \"bzip2 -dc ${kodebugbz2file} >${kodebugfile}\""] != 0} {
+    untested "failed bzip2 for ${kodebugbz2file}"
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# This test won't work properly if system debuginfo is installed.
+# Test message is suppressed by "" as otherwise we could print PASS+UNTESTED
+# result to gdb.sum making a false feeling the issue has been tested.
+gdb_test_no_output "set debug-file-directory" ""
+
+gdb_load ${kofile}
+
+set test "show architecture"
+gdb_test_multiple $test $test {
+    -re "\r\nThe target architecture is set automatically \\(currently powerpc:common64\\)\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "\r\nThe target architecture is set automatically \\(currently .*\\)\r\n$gdb_prompt $" {
+	untested "powerpc:common64 is not supported"
+    }
+}