On Mon, 2014-12-15 at 13:48 -0800, Josh Stone wrote: > On 12/13/2014 03:18 PM, Mark Wielaard wrote: > > On Thu, Dec 11, 2014 at 05:34:06PM -0800, Josh Stone wrote: > >> 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? > > Those look good.
Thanks, pushed. > I think src/elfcmp.c compare_Elf32_Word() is also wrong, as big u32 > values could wrap int subtraction. I didn't find any others. Ah, missed that Elf32_Word is unsigned. There is an assert that makes sure it is at least as wide as an int, but that isn't enough indeed. Proposed fix attached. Thanks, Mark
From 6c8781d9175900e321a8afe2c5073db68872e8e0 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <[email protected]> Date: Tue, 16 Dec 2014 11:04:55 +0100 Subject: [PATCH] elfcmp: Make sure Elf32_Word difference doesn't wrap around in int compare. Signed-off-by: Mark Wielaard <[email protected]> --- src/ChangeLog | 5 +++++ src/elfcmp.c | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 141b31f..1e612bf 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2014-12-16 Mark Wielaard <[email protected]> + + * elfcmp.c (compare_Elf32_Word): Make sure (unsigned) Elf32_Word + difference doesn't wrap around before returning as int. + 2014-12-11 Mark Wielaard <[email protected]> * readelf.c (print_debug_exception_table): Check TType base offset diff --git a/src/elfcmp.c b/src/elfcmp.c index c420019..d1008b3 100644 --- a/src/elfcmp.c +++ b/src/elfcmp.c @@ -811,8 +811,7 @@ compare_Elf32_Word (const void *p1, const void *p2) { const Elf32_Word *w1 = p1; const Elf32_Word *w2 = p2; - assert (sizeof (int) >= sizeof (*w1)); - return (int) *w1 - (int) *w2; + return *w1 < *w2 ? -1 : *w1 > *w2 ? 1 : 0; } static int -- 1.8.3.1
