[PATCH 1/3] libdw: Make srclines use a stable sort

Mark Wielaard mjw at redhat.com
Sat Dec 13 23:18:58 UTC 2014


On Thu, Dec 11, 2014 at 05:34:06PM -0800, Josh Stone wrote:
> BTW, I want to point out this change in compare_lines:
> 
> > -  return (*p1)->addr - (*p2)->addr;
> [...]
> > +  if (line1->addr != line2->addr)
> > +    return (line1->addr < line2->addr) ? -1 : 1;
> 
> Since addr is 64-bit unsigned, and comparison functions return int, it
> is possible for the difference to be so large that it wraps around.  You
> only need INT_MAX or more -- which probably doesn't happen often in ELF
> files, but it's plausible.
> 
> It might be worth auditing other qsort/tsearch comparison functions for
> similar wrapping possibilities.

I think you are right. I looked over all compare functions and two didn't
do as you suggest. The attached patch fixes those. Do that look correct?

Thanks,

Mark
-------------- next part --------------
>From 2c5e072b193572838cb3bbbc1f0ca7663d088c4a Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw at redhat.com>
Date: Sun, 14 Dec 2014 00:09:29 +0100
Subject: [PATCH] Guard against 64bit unsigned wrap around in (int) compare
 functions.

Dwarf_Adrr and Dwarf_Off are 64-bit unsigned, and comparison functions
used in qsort or tfind return int, it is possible for the difference to
be so large that it wraps around. Make sure to just return -1, 0 or 1
in compare_aranges and compare_cukey.

Signed-off-by: Mark Wielaard <mjw at redhat.com>
---
 libdw/ChangeLog          | 5 +++++
 libdw/dwarf_getaranges.c | 4 +++-
 libdwfl/ChangeLog        | 5 +++++
 libdwfl/cu.c             | 4 +++-
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 2fe5454..bae50a3 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2014-12-13  Mark Wielaard  <mjw at redhat.com>
+
+	* dwarf_getaranges.c (compare_aranges): Make sure Dwarf_Addr
+	difference doesn't wrap around before returning as int.
+
 2014-12-12  Mark Wielaard  <mjw at redhat.com>
 
 	* libdwP.h (struct Dwarf): Add fake_loc_cu.
diff --git a/libdw/dwarf_getaranges.c b/libdw/dwarf_getaranges.c
index 20ac7ec..4953af5 100644
--- a/libdw/dwarf_getaranges.c
+++ b/libdw/dwarf_getaranges.c
@@ -48,7 +48,9 @@ compare_aranges (const void *a, const void *b)
 {
   struct arangelist *const *p1 = a, *const *p2 = b;
   struct arangelist *l1 = *p1, *l2 = *p2;
-  return l1->arange.addr - l2->arange.addr;
+  if (l1->arange.addr != l2->arange.addr)
+    return (l1->arange.addr < l2->arange.addr) ? -1 : 1;
+  return 0;
 }
 
 int
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 99d555f..aa188a7 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,5 +1,10 @@
 2014-12-13  Mark Wielaard  <mjw at redhat.com>
 
+	* cu.c (cudie_offset): Make sure Dwarf_Off difference doesn't
+	wrap around before returning as int.
+
+2014-12-13  Mark Wielaard  <mjw at redhat.com>
+
 	* dwfl_module_getdwarf.c (find_dynsym): elf_getdata_rawchunk takes
 	a size_t, make sure it doesn't overflow.
 
diff --git a/libdwfl/cu.c b/libdwfl/cu.c
index 40b0201..5ce531b 100644
--- a/libdwfl/cu.c
+++ b/libdwfl/cu.c
@@ -162,7 +162,9 @@ cudie_offset (const struct dwfl_cu *cu)
 static int
 compare_cukey (const void *a, const void *b)
 {
-  return cudie_offset (a) - cudie_offset (b);
+  Dwarf_Off a_off = cudie_offset (a);
+  Dwarf_Off b_off = cudie_offset (b);
+  return (a_off < b_off) ? -1 : ((a_off > b_off) ? 1 : 0);
 }
 
 /* Intern the CU if necessary.  */
-- 
2.1.0



More information about the elfutils-devel mailing list