[PATCH][gdb] Fix gdb crash when reading core file

[Tom de Vries]

Hi,

Consider the test-case from this patch, compiled with O0.

The executable segfaults, and generates a core dump:
...
$ ./a.out
Segmentation fault (core dumped)
...

When loading the core file, limiting stack size to 4MB, gdb crashes:
...
$ ulimit -s 4096
$ gdb -batch ./a.out core.saved
[New LWP 19379]
Segmentation fault (core dumped)
...

The crash originates here in linux_vsyscall_range_raw, where we call alloca
with phdrs_size == 4194112 (roughly 4MB):
...
phdrs = (Elf_Internal_Phdr *) alloca (phdrs_size);
...

While for this test-case gdb runs fine with the system default stack limit of
8MB, there are cases reported of 12MB phdrs_size where gdb also crashes with
the system default stack limit.

Fix this by using xmalloc instead of alloca, which prevents the crash provided
the stack limit is at least 112kb.

Build and reg-tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb] Fix gdb crash when reading core file

2018-11-01 Tom de Vries <***@suse.de>

* linux-tdep.c (linux_vsyscall_range_raw): Use xmalloc to allocate
program headers.

* gdb.base/many-headers.c: New test.
* gdb.base/many-headers.exp: New file.

---
gdb/linux-tdep.c | 12 +++---
gdb/testsuite/gdb.base/many-headers.c | 33 ++++++++++++++++
gdb/testsuite/gdb.base/many-headers.exp | 67 +++++++++++++++++++++++++++++++++
3 files changed, 106 insertions(+), 6 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index c958c0dfe9..2c766808f0 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2269,7 +2269,6 @@ linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
the vDSO. */
if (!target_has_execution)
{
- Elf_Internal_Phdr *phdrs;
long phdrs_size;
int num_phdrs, i;

@@ -2277,16 +2276,17 @@ linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
if (phdrs_size == -1)
return 0;

- phdrs = (Elf_Internal_Phdr *) alloca (phdrs_size);
- num_phdrs = bfd_get_elf_phdrs (core_bfd, phdrs);
+ gdb::unique_xmalloc_ptr<Elf_Internal_Phdr>
+ phdrs ((Elf_Internal_Phdr *) xmalloc (phdrs_size));
+ num_phdrs = bfd_get_elf_phdrs (core_bfd, phdrs.get ());
if (num_phdrs == -1)
return 0;

for (i = 0; i < num_phdrs; i++)
- if (phdrs[i].p_type == PT_LOAD
- && phdrs[i].p_vaddr == range->start)
+ if (phdrs.get ()[i].p_type == PT_LOAD
+ && phdrs.get ()[i].p_vaddr == range->start)
{
- range->length = phdrs[i].p_memsz;
+ range->length = phdrs.get ()[i].p_memsz;
return 1;
}

diff --git a/gdb/testsuite/gdb.base/many-headers.c b/gdb/testsuite/gdb.base/many-headers.c
new file mode 100644
index 0000000000..57f82e1a6a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/many-headers.c
@@ -0,0 +1,33 @@
+#include <stdio.h>
+#include <malloc.h>
+#include <sys/mman.h>
+
+int
+main()
+{
+ char *ptr;
+ int ind, cnt;
+
+ cnt = 100000;
+ for (ind = 0; ind < cnt; ind++)
+ {
+ ptr = mmap(NULL, 100, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+ if (ptr == NULL)
+ {
+ fprintf(stderr, "Error allocating memory using mmap\n");
+ return -1;
+ }
+
+ ptr = mmap(NULL, 100, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+ if (ptr == NULL)
+ {
+ fprintf(stderr, "Error allocating memory using mmap\n");
+ return -1;
+ }
+ }
+
+ ptr = NULL;
+ *ptr = '\0';
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/many-headers.exp b/gdb/testsuite/gdb.base/many-headers.exp
new file mode 100644
index 0000000000..f02b291e36
--- /dev/null
+++ b/gdb/testsuite/gdb.base/many-headers.exp
@@ -0,0 +1,67 @@
+# Copyright 2018 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/>.
+
+# Test if gdb can read a core file with a program header size of 4MB, if the
+# stack size is limited to 4MB.
+
+if { [target_info gdb_protocol] != "" } {
+ # Even though the feature under features being tested are supported by
+ # gdbserver, the way this test is written doesn't make it easy with a
+ # remote target.
+ unsupported "not native"
+ return
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+ return -1
+}
+
+# Generate core file.
+set corefile [core_find $binfile]
+if {$corefile == ""} {
+ return 0
+}
+
+# Limit is in kb, so this is 4MB.
+set stack_limit 4096
+
+# Verify if we can set the stack limit.
+catch {
+ system [concat \
+ "(" \
+ "ulimit -s $stack_limit;" \
+ ")"]
+} msg
+if { "$msg" != "" } {
+ untested "Can't set stack limit"
+ return -1
+}
+
+# Run gdb with stack limit
+catch {
+ system [concat \
+ "(" \
+ "ulimit -s $stack_limit;" \
+ "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS -batch -core=$corefile" \
+ ")"]
+} msg
+set test "read core file"
+if { "$msg" == "" } {
+ pass "$test"
+} else {
+ fail "$test"
+}

[Tom Tromey]

Tom> 2018-11-01 Tom de Vries <***@suse.de>
Tom> * linux-tdep.c (linux_vsyscall_range_raw): Use xmalloc to allocate
Tom> program headers.
Tom> * gdb.base/many-headers.c: New test.
Tom> * gdb.base/many-headers.exp: New file.

Thanks for the patch.
This looks good. However, I think the new .c file should have a
copyright header. This is ok with that change.

Tom