]> asedeno.scripts.mit.edu Git - linux.git/commitdiff
vme: Fix integer overflow checking in vme_check_window()
authorDan Carpenter <dan.carpenter@oracle.com>
Sat, 30 Sep 2017 11:27:41 +0000 (14:27 +0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 4 Oct 2017 08:30:53 +0000 (10:30 +0200)
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@oracle.com>
Acked-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/vme/vme.c

index 6a3ead42aba876cef534b8e9acc34b30cf6cef3a..5b4c898d75099b63f528295d6cda25f31efd9857 100644 (file)
@@ -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: