The controversial part of this patch is that I've changed it so we now prevent integer overflows for VME_USER types and before we didn't. I view it as kernel-hardening. I looked at a couple places that used VME_USER types and they seemed pretty suspicious so I'm pretty sure preventing overflows here is a good idea. The most common problem which this function is for cases like VME_A16 where we don't put an upper bound on "size" so you could have "size" set to U64_MAX and a valid vme_base would overflow the "vme_base + size" into the valid range as well. In the VME_A64 case, the integer overflow checking doesn't work because "U64_MAX + 1" has an integer overflow and it's just a complicated way of saying zero. That VME_A64 case is sort of interesting as well because there is a VME_A64_MAX define which is set to "U64_MAX + 1". The compiler will never let anyone use it since it can't be stored in a u64 variable... With my patch it's now limited to just U64_MAX. Anyway, I put one integer overflow check at the start of the function and deleted all existing checks. Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c index 6a3ead42aba8..5b4c898d7509 100644 --- a/drivers/vme/vme.c +++ b/drivers/vme/vme.c @@ -208,29 +208,27 @@ int vme_check_window(u32 aspace, unsigned long long vme_base, { int retval = 0; + if (vme_base + size < size) + return -EINVAL; + switch (aspace) { case VME_A16: - if (((vme_base + size) > VME_A16_MAX) || - (vme_base > VME_A16_MAX)) + if (vme_base + size > VME_A16_MAX) retval = -EFAULT; break; case VME_A24: - if (((vme_base + size) > VME_A24_MAX) || - (vme_base > VME_A24_MAX)) + if (vme_base + size > VME_A24_MAX) retval = -EFAULT; break; case VME_A32: - if (((vme_base + size) > VME_A32_MAX) || - (vme_base > VME_A32_MAX)) + if (vme_base + size > VME_A32_MAX) retval = -EFAULT; break; case VME_A64: - if ((size != 0) && (vme_base > U64_MAX + 1 - size)) - retval = -EFAULT; + /* The VME_A64_MAX limit is actually U64_MAX + 1 */ break; case VME_CRCSR: - if (((vme_base + size) > VME_CRCSR_MAX) || - (vme_base > VME_CRCSR_MAX)) + if (vme_base + size > VME_CRCSR_MAX) retval = -EFAULT; break; case VME_USER1: _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel