|
 |
901878 |
commit b00a4fa78779ff0f304fa6cb34d49622679c86d4
|
|
|
901878 |
Author: Mark Wielaard <mjw@redhat.com>
|
|
|
901878 |
Date: Thu Sep 3 10:50:58 2015 +0200
|
|
|
901878 |
|
|
|
901878 |
readelf: handle_core_item large right shift triggers undefined behaviour.
|
|
|
901878 |
|
|
|
901878 |
The problem is this:
|
|
|
901878 |
|
|
|
901878 |
int n = ffs (w);
|
|
|
901878 |
w >>= n;
|
|
|
901878 |
|
|
|
901878 |
The intent is to shift away up to (and including) the first least
|
|
|
901878 |
significant bit in w. But w is an unsigned int, so 32 bits. And the
|
|
|
901878 |
least significant bit could be bit 32 (ffs counts from 1). Unfortunately
|
|
|
901878 |
a right shift equal to (or larger than) the length in bits of the left
|
|
|
901878 |
hand operand is undefined behaviour. We expect w to be zero afterwards.
|
|
|
901878 |
Which would terminate the while loop in the function. But since it is
|
|
|
901878 |
undefined behaviour anything can happen. In this case, what will actually
|
|
|
901878 |
happen is that w is unchanged, causing an infinite loop...
|
|
|
901878 |
|
|
|
901878 |
gcc -fsanitize=undefined will catch and warn about this when w = 0x80000000
|
|
|
901878 |
|
|
|
901878 |
https://bugzilla.redhat.com/show_bug.cgi?id=1259259
|
|
|
901878 |
|
|
|
901878 |
Signed-off-by: Mark Wielaard <mjw@redhat.com>
|
|
|
901878 |
|
|
|
901878 |
diff --git a/src/readelf.c b/src/readelf.c
|
|
|
901878 |
index d3c2b6b..aab8b5c 100644
|
|
|
901878 |
--- a/src/readelf.c
|
|
|
901878 |
+++ b/src/readelf.c
|
|
|
901878 |
@@ -8474,8 +8474,16 @@ handle_core_item (Elf *core, const Ebl_Core_Item *item, const void *desc,
|
|
|
901878 |
unsigned int w = negate ? ~*i : *i;
|
|
|
901878 |
while (w != 0)
|
|
|
901878 |
{
|
|
|
901878 |
- int n = ffs (w);
|
|
|
901878 |
- w >>= n;
|
|
|
901878 |
+ /* Note that a right shift equal to (or greater than)
|
|
|
901878 |
+ the number of bits of w is undefined behaviour. In
|
|
|
901878 |
+ particular when the least significant bit is bit 32
|
|
|
901878 |
+ (w = 0x8000000) then w >>= n is undefined. So
|
|
|
901878 |
+ explicitly handle that case separately. */
|
|
|
901878 |
+ unsigned int n = ffs (w);
|
|
|
901878 |
+ if (n < sizeof (w) * 8)
|
|
|
901878 |
+ w >>= n;
|
|
|
901878 |
+ else
|
|
|
901878 |
+ w = 0;
|
|
|
901878 |
bit += n;
|
|
|
901878 |
|
|
|
901878 |
if (lastbit != 0 && lastbit + 1 == bit)
|