diff mbox

Avoid segfault on invalid directory table

Message ID 6c10a81a-b9fe-cd6b-0adc-85fc7b596c1d@fb.com
State New, archived
Headers

Commit Message

Padraig Brady March 24, 2017, 4:09 a.m. UTC 
  gdb was segfaulting during backtrace on a binary here,
where fe->dir_index parsed from the dwarf info was seen to
access beyond the provided include_dirs array.
The attached bounds the access to entries actually written to the array,
and was verified to output the backtrace correctly.

cheers,
Pádraig

Comments

Pedro Alves March 24, 2017, 3:26 p.m. UTC | #1 
Hi Pádraig,

On 03/24/2017 04:09 AM, Padraig Brady wrote:
> gdb was segfaulting during backtrace on a binary here,
> where fe->dir_index parsed from the dwarf info was seen to
> access beyond the provided include_dirs array.
> The attached bounds the access to entries actually written to the array,
> and was verified to output the backtrace correctly.
> 

Thanks, I pushed it.

Note that AFAICS, Facebook has as couple copyright assignments in place,
but none covers you for GDB.  The patch was sufficiently small that it
shouldn't be a problem this time.  But it'd be appreciated if that
was sorted out if you plan on contributing more.

And also, could you give this patch a try? :

  https://sourceware.org/ml/gdb-patches/2017-03/msg00444.html

(I see now that I mistakenly named you as author of that commit.
Sorry about that.  I've fixed it locally.  :-) )

Thanks,
Pedro Alves
Padraig Brady March 24, 2017, 5:38 p.m. UTC | #2 
On 03/24/2017 08:26 AM, Pedro Alves wrote:
> Hi Pádraig,
>
> On 03/24/2017 04:09 AM, Padraig Brady wrote:
>> gdb was segfaulting during backtrace on a binary here,
>> where fe->dir_index parsed from the dwarf info was seen to
>> access beyond the provided include_dirs array.
>> The attached bounds the access to entries actually written to the array,
>> and was verified to output the backtrace correctly.
>>
> Thanks, I pushed it.
>
> Note that AFAICS, Facebook has as couple copyright assignments in place,
> but none covers you for GDB.  The patch was sufficiently small that it
> shouldn't be a problem this time.  But it'd be appreciated if that
> was sorted out if you plan on contributing more.
>
> And also, could you give this patch a try? :
>
>   https://urldefense.proofpoint.com/v2/url?u=https-3A__sourceware.org_ml_gdb-2Dpatches_2017-2D03_msg00444.html&d=DwIDaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=IRyzGZVy_JZ1hqY1erVdUg&m=vs5e-LmOdHODMsuSCzBW-ZmATTGCvlUF_g5zTPBZOrg&s=xgu4H5IZ76MWdl6vuMFxqYhOV_jshZnaDrB6bw6Gd94&e= 
>
> (I see now that I mistakenly named you as author of that commit.
> Sorry about that.  I've fixed it locally.  :-) )

That patch produces a good backtrace against the problematic binary.

cheers,
Pádraig
diff mbox

Patch

From bc176bf7052db2242b2fb6f10dcbfe15c5a3e7e0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbrady@fb.com>
Date: Thu, 23 Mar 2017 20:33:47 -0700
Subject: [PATCH] avoid segfault on invalid directory table

This was seen to avoid a segfault when doing a
backtrace in certain binaries.

gdb/
* dwarf2read.c (setup_type_unit_groups): Ensure dir_index
doesn't reference beyond the provided include_dirs.
to 'lh->include_dirs' before accessing to it.
(psymtab_include_file_name): Likewise.
(dwarf_decode_lines_1): Likewise.
(dwarf_decode_lines): Likewise.
(file_file_name): Likewise.
---
 gdb/dwarf2read.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b3ea52b..519550b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9416,7 +9416,8 @@  setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu)
 	  const char *dir = NULL;
 	  struct file_entry *fe = &lh->file_names[i];
 
-	  if (fe->dir_index && lh->include_dirs != NULL)
+	  if (fe->dir_index && lh->include_dirs != NULL
+	      && (fe->dir_index - 1) < lh->num_include_dirs)
 	    dir = lh->include_dirs[fe->dir_index - 1];
 	  dwarf2_start_subfile (fe->name, dir);
 
@@ -17985,7 +17986,8 @@  psymtab_include_file_name (const struct line_header *lh, int file_index,
   char *copied_name = NULL;
   int file_is_pst;
 
-  if (fe.dir_index && lh->include_dirs != NULL)
+  if (fe.dir_index && lh->include_dirs != NULL
+      && (fe.dir_index - 1) < lh->num_include_dirs)
     dir_name = lh->include_dirs[fe.dir_index - 1];
 
   if (!IS_ABSOLUTE_PATH (include_name)
@@ -18366,7 +18368,8 @@  dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
           struct file_entry *fe = &lh->file_names[state_machine.file - 1];
           const char *dir = NULL;
 
-          if (fe->dir_index && lh->include_dirs != NULL)
+          if (fe->dir_index && lh->include_dirs != NULL
+              && (fe->dir_index - 1) < lh->num_include_dirs)
             dir = lh->include_dirs[fe->dir_index - 1];
 
 	  dwarf2_start_subfile (fe->name, dir);
@@ -18529,7 +18532,8 @@  dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 		else
 		  {
 		    fe = &lh->file_names[state_machine.file - 1];
-		    if (fe->dir_index && lh->include_dirs != NULL)
+		    if (fe->dir_index && lh->include_dirs != NULL
+		        && (fe->dir_index - 1) < lh->num_include_dirs)
 		      dir = lh->include_dirs[fe->dir_index - 1];
 		    if (record_lines_p)
 		      {
@@ -18671,7 +18675,8 @@  dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 	  struct file_entry *fe;
 
 	  fe = &lh->file_names[i];
-	  if (fe->dir_index && lh->include_dirs != NULL)
+	  if (fe->dir_index && lh->include_dirs != NULL
+	      && (fe->dir_index - 1) < lh->num_include_dirs)
 	    dir = lh->include_dirs[fe->dir_index - 1];
 	  dwarf2_start_subfile (fe->name, dir);
 
@@ -21380,7 +21385,8 @@  file_file_name (int file, struct line_header *lh)
       struct file_entry *fe = &lh->file_names[file - 1];
 
       if (IS_ABSOLUTE_PATH (fe->name) || fe->dir_index == 0
-	  || lh->include_dirs == NULL)
+	  || lh->include_dirs == NULL
+	  || (fe->dir_index - 1) >= lh->num_include_dirs)
         return xstrdup (fe->name);
       return concat (lh->include_dirs[fe->dir_index - 1], SLASH_STRING,
 		     fe->name, (char *) NULL);
-- 
2.5.5