Changeset 52575 in vbox

Timestamp:

Sep 3, 2014 7:36:27 AM (10 years ago)

Author:

vboxsync

Message:

SUPDrv: Several fixes, thanks to Mathias Krause.

Location:

trunk/src/VBox/HostDrivers/Support

Files:

• SUPDrv.c (modified) (3 diffs)
• SUPDrvInternal.h (modified) (1 diff)
• darwin/SUPDrv-darwin.cpp (modified) (2 diffs)
• freebsd/SUPDrv-freebsd.c (modified) (2 diffs)
• linux/SUPDrv-linux.c (modified) (2 diffs)
• os2/SUPDrv-os2.cpp (modified) (1 diff)
• solaris/SUPDrv-solaris.c (modified) (1 diff)
• win/SUPDrv-win.cpp (modified) (2 diffs)

trunk/src/VBox/HostDrivers/Support/SUPDrv.c

@@ -2293 +2293 @@
  * @param   pReqHdr     The request header.
  */
-int VBOXCALL supdrvIOCtl(uintptr_t uIOCtl, PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession, PSUPREQHDR pReqHdr)
+int VBOXCALL supdrvIOCtl(uintptr_t uIOCtl, PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession, PSUPREQHDR pReqHdr, size_t cbReq)
 {
     int rc;
@@ -2301 +2301 @@
      * Validate the request.
      */
-    /* this first check could probably be omitted as its also done by the OS specific code... */
+    if (RT_UNLIKELY(cbReq < sizeof(*pReqHdr)))
+    {
+        OSDBGPRINT(("vboxdrv: Bad ioctl request size; cbReq=%#lx\n", (long)cbReq));
+        VBOXDRV_IOCTL_RETURN(pSession, uIOCtl, pReqHdr, VERR_INVALID_PARAMETER, VINF_SUCCESS);
+        return VERR_INVALID_PARAMETER;
+    }
     if (RT_UNLIKELY(   (pReqHdr->fFlags & SUPREQHDR_FLAGS_MAGIC_MASK) != SUPREQHDR_FLAGS_MAGIC
                     || pReqHdr->cbIn < sizeof(*pReqHdr)
-                    || pReqHdr->cbOut < sizeof(*pReqHdr)))
+                    || pReqHdr->cbIn > cbReq
+                    || pReqHdr->cbOut < sizeof(*pReqHdr)
+                    || pReqHdr->cbOut > cbReq))
     {
         OSDBGPRINT(("vboxdrv: Bad ioctl request header; cbIn=%#lx cbOut=%#lx fFlags=%#lx\n",
@@ -4448 +4455 @@
             &&  !memcmp(pImage->szName, pReq->u.In.szName, cchName))
         {
-            /** @todo check cbImageBits and cbImageWithTabs here, if they differs that indicates that the images are different. */
-            pImage->cUsage++;
-            pReq->u.Out.pvImageBase   = pImage->pvImage;
-            pReq->u.Out.fNeedsLoading = pImage->uState == SUP_IOCTL_LDR_OPEN;
-            pReq->u.Out.fNativeLoader = pImage->fNative;
-            supdrvLdrAddUsage(pSession, pImage);
+            if (RT_LIKELY(pImage->cUsage < UINT32_MAX / 2U))
+            {
+                /** @todo check cbImageBits and cbImageWithTabs here, if they differs that indicates that the images are different. */
+                pImage->cUsage++;
+                pReq->u.Out.pvImageBase   = pImage->pvImage;
+                pReq->u.Out.fNeedsLoading = pImage->uState == SUP_IOCTL_LDR_OPEN;
+                pReq->u.Out.fNativeLoader = pImage->fNative;
+                supdrvLdrAddUsage(pSession, pImage);
+                supdrvLdrUnlock(pDevExt);
+                return VINF_SUCCESS;
+            }
             supdrvLdrUnlock(pDevExt);
-            return VINF_SUCCESS;
+            Log(("supdrvIOCtl_LdrOpen: To many existing references to '%s'!\n", pReq->u.In.szName));
+            return VERR_INTERNAL_ERROR_3; /** @todo add VERR_TOO_MANY_REFERENCES */
         }
     }

trunk/src/VBox/HostDrivers/Support/SUPDrvInternal.h

@@ -823 +823 @@
 *******************************************************************************/
 /* SUPDrv.c */
-int  VBOXCALL   supdrvIOCtl(uintptr_t uIOCtl, PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession, PSUPREQHDR pReqHdr);
+int  VBOXCALL   supdrvIOCtl(uintptr_t uIOCtl, PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession, PSUPREQHDR pReqHdr, size_t cbReq);
 int  VBOXCALL   supdrvIOCtlFast(uintptr_t uIOCtl, VMCPUID idCpu, PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession);
 int  VBOXCALL   supdrvIDC(uintptr_t uIOCtl, PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession, PSUPDRVIDCREQHDR pReqHdr);

trunk/src/VBox/HostDrivers/Support/darwin/SUPDrv-darwin.cpp

@@ -689 +689 @@
             return rc;
         }
+        if (Hdr.cbIn < cbReq)
+            RT_BZERO((uint8_t *)pHdr + Hdr.cbIn, cbReq - Hdr.cbIn)
     }
     else
@@ -699 +701 @@
      * Process the IOCtl.
      */
-    int rc = supdrvIOCtl(iCmd, &g_DevExt, pSession, pHdr);
+    int rc = supdrvIOCtl(iCmd, &g_DevExt, pSession, pHdr, cbReq);
     if (RT_LIKELY(!rc))
     {

trunk/src/VBox/HostDrivers/Support/freebsd/SUPDrv-freebsd.c

@@ -411 +411 @@
             return rc;
         }
+        if (Hdr.cbIn < cbReq)
+            RT_BZERO((uint8_t *)pHdr + Hdr.cbIn, cbReq - Hdr.cbIn)
     }
     else
@@ -421 +423 @@
      * Process the IOCtl.
      */
-    int rc = supdrvIOCtl(ulCmd, &g_VBoxDrvFreeBSDDevExt, pSession, pHdr);
+    int rc = supdrvIOCtl(ulCmd, &g_VBoxDrvFreeBSDDevExt, pSession, pHdr, cbReq);
     if (RT_LIKELY(!rc))
     {

trunk/src/VBox/HostDrivers/Support/linux/SUPDrv-linux.c

@@ -719 +719 @@
         return -E2BIG;
     }
-    if (RT_UNLIKELY(cbBuf != _IOC_SIZE(uCmd) && _IOC_SIZE(uCmd)))
+    if (RT_UNLIKELY(_IOC_SIZE(uCmd) ? cbBuf != _IOC_SIZE(uCmd) : Hdr.cbIn < sizeof(Hdr)))
     {
         Log(("VBoxDrvLinuxIOCtl: bad ioctl cbBuf=%#x _IOC_SIZE=%#x; uCmd=%#x.\n", cbBuf, _IOC_SIZE(uCmd), uCmd));
@@ -736 +736 @@
         return -EFAULT;
     }
+    if (Hdr.cbIn < cbBuf)
+        RT_BZERO((uint8_t *)pHdr + Hdr.cbIn, cbBuf - Hdr.cbIn)
 
     /*
      * Process the IOCtl.
      */
-    rc = supdrvIOCtl(uCmd, &g_DevExt, pSession, pHdr);
+    rc = supdrvIOCtl(uCmd, &g_DevExt, pSession, pHdr, cbBuf);
 
     /*

trunk/src/VBox/HostDrivers/Support/os2/SUPDrv-os2.cpp

@@ -335 +335 @@
                  * Process the IOCtl.
                  */
-                rc = supdrvIOCtl(iFunction, &g_DevExt, pSession, pHdr);
+                rc = supdrvIOCtl(iFunction, &g_DevExt, pSession, pHdr, cbReq);
             }
             else

trunk/src/VBox/HostDrivers/Support/solaris/SUPDrv-solaris.c

@@ -816 +816 @@
      * Process the IOCtl.
      */
-    rc = supdrvIOCtl(iCmd, &g_DevExt, pSession, pHdr);
+    rc = supdrvIOCtl(iCmd, &g_DevExt, pSession, pHdr, cbBuf);
 
     /*

trunk/src/VBox/HostDrivers/Support/win/SUPDrv-win.cpp

@@ -1009 +1009 @@
                          * Now call the common code to do the real work.
                          */
-                        rc = supdrvIOCtl(uCmd, pDevExt, pSession, pHdr);
+                        rc = supdrvIOCtl(uCmd, pDevExt, pSession, pHdr, cbBuf);
                         if (RT_SUCCESS(rc))
                         {
@@ -1159 +1159 @@
                 &&  pStack->Parameters.DeviceIoControl.OutputBufferLength ==  pHdr->cbOut)
             {
+                /* Zero extra output bytes to make sure we don't leak anything. */
+                if (pHdr->cbIn < pHdr->cbOut)
+                    RtlZeroMemory((uint8_t *)pHdr + pHdr->cbIn, pHdr->cbOut - pHdr->cbIn);
+
                 /*
                  * Do the job.
                  */
-                rc = supdrvIOCtl(pStack->Parameters.DeviceIoControl.IoControlCode, pDevExt, pSession, pHdr);
+                rc = supdrvIOCtl(pStack->Parameters.DeviceIoControl.IoControlCode, pDevExt, pSession, pHdr,
+                                 RT_MAX(pHdr->cbIn, pHdr->cbOut));
                 if (!rc)
                 {