Changeset 19091 in vbox

Timestamp:

Apr 21, 2009 8:57:38 PM (16 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

46239

Message:

Additions/common and linux: don'\t try to lock zero-length buffers when we do HGCM copies

Location:

trunk/src/VBox/Additions

Files:

• common/VBoxGuestLib/SysHlp.cpp (modified) (3 diffs)
• linux/module/vboxmod.c (modified) (3 diffs)

trunk/src/VBox/Additions/common/VBoxGuestLib/SysHlp.cpp

@@ -36 +36 @@
     int rc = VINF_SUCCESS;
 
+    /* Ugly edge case - zero size buffers shouldn't be locked. */
+    if (u32Size == 0)
+    {
+        *ppvCtx = NIL_RTR0MEMOBJ;
+        return VINF_SUCCESS;
+    }
 #ifdef RT_OS_WINDOWS
     PMDL pMdl = IoAllocateMdl (pv, u32Size, FALSE, FALSE, NULL);
@@ -68 +74 @@
     /** @todo r=frank: Linux: pv is at least in some cases, e.g. with VBoxMapFolder,
      *  an R0 address -- the memory was allocated with kmalloc(). I don't know
-     *  if this is true in any case. */
+     *  if this is true in any case.
+     * r=michael: on Linux, we sometimes have R3 addresses (e.g. shared
+     *  clipboard) and sometimes R0 (e.g. shared folders).  We really ought
+     *  to have two separate paths here - at any rate, Linux R0 shouldn't 
+     *  end up calling this API.  In practice, Linux R3 does it's own thing
+     *  before winding up in the R0 path - which calls this stub API.
+     */
     NOREF(ppvCtx);
     NOREF(pv);
@@ -92 +104 @@
     NOREF(u32Size);
 
+    /* Ugly edge case - zero size buffers aren't be locked. */
+    if (pvCtx == NIL_RTR0MEMOBJ)
+        return;
 #ifdef RT_OS_WINDOWS
     PMDL pMdl = (PMDL)pvCtx;

trunk/src/VBox/Additions/linux/module/vboxmod.c

@@ -442 +442 @@
 
     AssertPtrReturn(ppBuf, -EINVAL);
-    AssertPtrReturn(pUser, -EINVAL);
 
     pBuf = RTMemAlloc(sizeof(*pBuf));
@@ -449 +448 @@
     if (rc >= 0)
     {
-        pKernel = RTMemAlloc(cb);
-        if (pKernel == NULL)
-            rc = -ENOMEM;
-    }
-    if (   rc >= 0
-        && copy
-        && copy_from_user(pKernel, pUser, cb) != 0)
-        rc = -EFAULT;
+        if (cb > 0)
+        {
+            pKernel = RTMemAlloc(cb);
+            if (pKernel == NULL)
+                rc = -ENOMEM;
+            if (   rc >= 0
+                && copy
+                && copy_from_user(pKernel, pUser, cb) != 0)
+                rc = -EFAULT;
+        }
+        else
+            /* Empty buffers are allowed, but then the user pointer is not
+             * required to be valid, and we definitely don't want to copy
+             * anything. */
+            pKernel = NULL;
+    }
     if (rc >= 0)
     {
@@ -478 +485 @@
     int rc = 0;
     AssertPtrReturn(pBuf, -EINVAL);
-    if (copy && copy_to_user(pBuf->pUser, pBuf->pKernel, pBuf->cb) != 0)
+    if ((pBuf->cb > 0)
+        && copy
+        && copy_to_user(pBuf->pUser, pBuf->pKernel, pBuf->cb) != 0)
         rc = -EFAULT;
     RTMemFree(pBuf->pKernel);  /* We want to do this whatever the outcome. */