[PATCH 2/2] enum: fix UB when rshifting by full width

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Shifting by an amount greater or equal than the width
of the type is Undefined Behaviour. In the present case,
when type_is_ok() is called with a type as wide as an
ullong (64 bits here), the bounds are shifted by 64
which is UB and at execution (on x86) the value is simply
unchanged (since the shift is done with the amount modulo 63).

This, of course, doesn't give the expected result and
as consequence valid enums can have an invalid base type
(bad_ctype).

Fix this by doing the shift with a small helper which
test if the amount is of the maximum width and return
zero if it is the case.

NB. Doing the shift in two steps could also be a solution
  and maybe also some clever tricks but since this code is
  no way critical performance-wise the solution here is
  good and has the merit of being very explicit.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
---
 parse.c                    | 16 ++++++++++++++--
 validation/bug-rshift-ub.c |  1 -
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/parse.c b/parse.c
index d94c53a22..32ccaa039 100644
--- a/parse.c
+++ b/parse.c
@@ -793,6 +793,18 @@ static void lower_boundary(Num *n, Num *v)
 		n->y = v->y;
 }
 
+///
+// rshift() - a 'safe' right shift.
+//	This allow to use a shift amount as big (or bigger)
+//	than the width of the value to be shifted, in which case
+//	the result is, of course, 0.
+static unsigned long long rshift(unsigned long long val, unsigned int n)
+{
+	if (n >= (sizeof(val) * 8))
+		return 0;
+	return val >> n;
+}
+
 static int type_is_ok(struct symbol *type, Num *upper, Num *lower)
 {
 	int shift = type->bit_size;
@@ -800,9 +812,9 @@ static int type_is_ok(struct symbol *type, Num *upper, Num *lower)
 
 	if (!is_unsigned)
 		shift--;
-	if (upper->x == 0 && upper->y >> shift)
+	if (upper->x == 0 && rshift(upper->y, shift))
 		return 0;
-	if (lower->x == 0 || (!is_unsigned && (~lower->y >> shift) == 0))
+	if (lower->x == 0 || (!is_unsigned && rshift(~lower->y, shift) == 0))
 		return 1;
 	return 0;
 }
diff --git a/validation/bug-rshift-ub.c b/validation/bug-rshift-ub.c
index 3a5f3a577..7654abbd8 100644
--- a/validation/bug-rshift-ub.c
+++ b/validation/bug-rshift-ub.c
@@ -6,7 +6,6 @@ static enum a a = A;
 
 /*
  * check-name: bug-rshift-ub
- * check-known-to-fail
  * check-description:
  *	This test trigger(ed) a bug on x86 caused by a
  *	full width shift (which is UB), expecting to get
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux