Changeset 78465 in vbox for trunk/src/VBox/Additions

Timestamp:

May 10, 2019 7:11:29 PM (6 years ago)

Author:

vboxsync

Message:

winnt/vboxsf: Converted file & directory removal code to new driver/HGCM interface. bugref:9172

File:

• trunk/src/VBox/Additions/WINNT/SharedFolders/driver/path.cpp (modified) (6 diffs)

trunk/src/VBox/Additions/WINNT/SharedFolders/driver/path.cpp

@@ -695 +695 @@
 
 /**
- * Closes an opened file handle of a MRX_VBOX_FOBX.
+ * Ensures the FCBx doesn't have dangling pointers to @a pVBoxFobx.
  *
- * Updates file attributes if necessary.
- *
- * Used by VBoxMRxCloseSrvOpen, vbsfNtRemove and vbsfNtRename.
+ * This isn't strictly speaking needed, as nobody currently dereference these
+ * pointers, however better keeping things neath and tidy.
  */
-NTSTATUS vbsfNtCloseFileHandle(PMRX_VBOX_NETROOT_EXTENSION pNetRootExtension,
-                               PMRX_VBOX_FOBX pVBoxFobx,
-                               PVBSFNTFCBEXT pVBoxFcbx)
-{
-    if (pVBoxFobx->hFile == SHFL_HANDLE_NIL)
-    {
-        Log(("VBOXSF: vbsfCloseFileHandle: SHFL_HANDLE_NIL\n"));
-        return STATUS_SUCCESS;
-    }
-
-    Log(("VBOXSF: vbsfCloseFileHandle: 0x%RX64, fTimestampsUpdatingSuppressed = %#x, fTimestampsImplicitlyUpdated = %#x\n",
-         pVBoxFobx->hFile, pVBoxFobx->fTimestampsUpdatingSuppressed, pVBoxFobx->fTimestampsImplicitlyUpdated));
-
-    /*
-     * We allocate a single request buffer for the timestamp updating and the closing
-     * to save time (at the risk of running out of heap, but whatever).
-     */
-    union MyCloseAndInfoReq
-    {
-        VBOXSFCLOSEREQ   Close;
-        VBOXSFOBJINFOREQ Info;
-    } *pReq = (union MyCloseAndInfoReq *)VbglR0PhysHeapAlloc(sizeof(*pReq));
-    if (pReq)
-        RT_ZERO(*pReq);
-    else
-        return STATUS_INSUFF_SERVER_RESOURCES;
-
-    /*
-     * Restore timestamp that we may implicitly been updated via this handle
-     * after the user explicitly set them or turn off implict updating (the -1 value).
-     *
-     * Note! We ignore the status of this operation.
-     */
-    Assert(pVBoxFcbx);
-    uint8_t fUpdateTs = pVBoxFobx->fTimestampsUpdatingSuppressed & pVBoxFobx->fTimestampsImplicitlyUpdated;
-    if (fUpdateTs)
-    {
-        /** @todo skip this if the host is windows and fTimestampsUpdatingSuppressed == fTimestampsSetByUser */
-        /** @todo pass -1 timestamps thru so we can always skip this on windows hosts! */
-        if (   (fUpdateTs & VBOX_FOBX_F_INFO_LASTACCESS_TIME)
-            && pVBoxFcbx->pFobxLastAccessTime == pVBoxFobx)
-            pReq->Info.ObjInfo.AccessTime        = pVBoxFobx->Info.AccessTime;
-        else
-            fUpdateTs &= ~VBOX_FOBX_F_INFO_LASTACCESS_TIME;
-
-        if (   (fUpdateTs & VBOX_FOBX_F_INFO_LASTWRITE_TIME)
-            && pVBoxFcbx->pFobxLastWriteTime  == pVBoxFobx)
-            pReq->Info.ObjInfo.ModificationTime  = pVBoxFobx->Info.ModificationTime;
-        else
-            fUpdateTs &= ~VBOX_FOBX_F_INFO_LASTWRITE_TIME;
-
-        if (   (fUpdateTs & VBOX_FOBX_F_INFO_CHANGE_TIME)
-            && pVBoxFcbx->pFobxChangeTime     == pVBoxFobx)
-            pReq->Info.ObjInfo.ChangeTime        = pVBoxFobx->Info.ChangeTime;
-        else
-            fUpdateTs &= ~VBOX_FOBX_F_INFO_CHANGE_TIME;
-        if (fUpdateTs)
-        {
-            Log(("VBOXSF: vbsfCloseFileHandle: Updating timestamp: %#x\n", fUpdateTs));
-            int vrc = VbglR0SfHostReqSetObjInfo(pNetRootExtension->map.root, &pReq->Info, pVBoxFobx->hFile);
-            if (RT_FAILURE(vrc))
-                Log(("VBOXSF: vbsfCloseFileHandle: VbglR0SfHostReqSetObjInfo failed for fUpdateTs=%#x: %Rrc\n", fUpdateTs, vrc));
-            RT_NOREF(vrc);
-        }
-        else
-            Log(("VBOXSF: vbsfCloseFileHandle: no timestamp needing updating\n"));
-    }
-
-    /* This isn't strictly necessary, but best to keep things clean. */
+DECLINLINE(void) vbsfNtCleanupFcbxTimestampRefsOnClose(PMRX_VBOX_FOBX pVBoxFobx, PVBSFNTFCBEXT pVBoxFcbx)
+{
     pVBoxFobx->fTimestampsSetByUser          = 0;
     pVBoxFobx->fTimestampsUpdatingSuppressed = 0;
@@ -779 +711 @@
     if (pVBoxFcbx->pFobxChangeTime     == pVBoxFobx)
         pVBoxFcbx->pFobxChangeTime     = NULL;
+}
+
+/**
+ * Closes an opened file handle of a MRX_VBOX_FOBX.
+ *
+ * Updates file attributes if necessary.
+ *
+ * Used by VBoxMRxCloseSrvOpen and vbsfNtRename.
+ */
+NTSTATUS vbsfNtCloseFileHandle(PMRX_VBOX_NETROOT_EXTENSION pNetRootExtension,
+                               PMRX_VBOX_FOBX pVBoxFobx,
+                               PVBSFNTFCBEXT pVBoxFcbx)
+{
+    if (pVBoxFobx->hFile == SHFL_HANDLE_NIL)
+    {
+        Log(("VBOXSF: vbsfCloseFileHandle: SHFL_HANDLE_NIL\n"));
+        return STATUS_SUCCESS;
+    }
+
+    Log(("VBOXSF: vbsfCloseFileHandle: 0x%RX64, fTimestampsUpdatingSuppressed = %#x, fTimestampsImplicitlyUpdated = %#x\n",
+         pVBoxFobx->hFile, pVBoxFobx->fTimestampsUpdatingSuppressed, pVBoxFobx->fTimestampsImplicitlyUpdated));
+
+    /*
+     * We allocate a single request buffer for the timestamp updating and the closing
+     * to save time (at the risk of running out of heap, but whatever).
+     */
+    union MyCloseAndInfoReq
+    {
+        VBOXSFCLOSEREQ   Close;
+        VBOXSFOBJINFOREQ Info;
+    } *pReq = (union MyCloseAndInfoReq *)VbglR0PhysHeapAlloc(sizeof(*pReq));
+    if (pReq)
+        RT_ZERO(*pReq);
+    else
+        return STATUS_INSUFF_SERVER_RESOURCES;
+
+    /*
+     * Restore timestamp that we may implicitly been updated via this handle
+     * after the user explicitly set them or turn off implict updating (the -1 value).
+     *
+     * Note! We ignore the status of this operation.
+     */
+    Assert(pVBoxFcbx);
+    uint8_t fUpdateTs = pVBoxFobx->fTimestampsUpdatingSuppressed & pVBoxFobx->fTimestampsImplicitlyUpdated;
+    if (fUpdateTs)
+    {
+        /** @todo skip this if the host is windows and fTimestampsUpdatingSuppressed == fTimestampsSetByUser */
+        /** @todo pass -1 timestamps thru so we can always skip this on windows hosts! */
+        if (   (fUpdateTs & VBOX_FOBX_F_INFO_LASTACCESS_TIME)
+            && pVBoxFcbx->pFobxLastAccessTime == pVBoxFobx)
+            pReq->Info.ObjInfo.AccessTime        = pVBoxFobx->Info.AccessTime;
+        else
+            fUpdateTs &= ~VBOX_FOBX_F_INFO_LASTACCESS_TIME;
+
+        if (   (fUpdateTs & VBOX_FOBX_F_INFO_LASTWRITE_TIME)
+            && pVBoxFcbx->pFobxLastWriteTime  == pVBoxFobx)
+            pReq->Info.ObjInfo.ModificationTime  = pVBoxFobx->Info.ModificationTime;
+        else
+            fUpdateTs &= ~VBOX_FOBX_F_INFO_LASTWRITE_TIME;
+
+        if (   (fUpdateTs & VBOX_FOBX_F_INFO_CHANGE_TIME)
+            && pVBoxFcbx->pFobxChangeTime     == pVBoxFobx)
+            pReq->Info.ObjInfo.ChangeTime        = pVBoxFobx->Info.ChangeTime;
+        else
+            fUpdateTs &= ~VBOX_FOBX_F_INFO_CHANGE_TIME;
+        if (fUpdateTs)
+        {
+            Log(("VBOXSF: vbsfCloseFileHandle: Updating timestamp: %#x\n", fUpdateTs));
+            int vrc = VbglR0SfHostReqSetObjInfo(pNetRootExtension->map.root, &pReq->Info, pVBoxFobx->hFile);
+            if (RT_FAILURE(vrc))
+                Log(("VBOXSF: vbsfCloseFileHandle: VbglR0SfHostReqSetObjInfo failed for fUpdateTs=%#x: %Rrc\n", fUpdateTs, vrc));
+            RT_NOREF(vrc);
+        }
+        else
+            Log(("VBOXSF: vbsfCloseFileHandle: no timestamp needing updating\n"));
+    }
+
+    vbsfNtCleanupFcbxTimestampRefsOnClose(pVBoxFobx, pVBoxFcbx);
 
     /*
@@ -794 +804 @@
 }
 
+/**
+ * @note We don't collapse opens, this is called whenever a handle is closed.
+ */
 NTSTATUS VBoxMRxCloseSrvOpen(IN PRX_CONTEXT RxContext)
 {
-    NTSTATUS Status = STATUS_SUCCESS;
-
     RxCaptureFcb;
     RxCaptureFobx;
@@ -805 +816 @@
     PMRX_SRV_OPEN pSrvOpen = capFobx->pSrvOpen;
 
-    PUNICODE_STRING RemainingName = NULL;
 
     Log(("VBOXSF: MRxCloseSrvOpen: capFcb = %p, capFobx = %p, pVBoxFobx = %p, pSrvOpen = %p\n",
           capFcb, capFobx, pVBoxFobx, pSrvOpen));
 
-    RemainingName = pSrvOpen->pAlreadyPrefixedName;
-
+#ifdef LOG_ENABLED
+    PUNICODE_STRING pRemainingName = pSrvOpen->pAlreadyPrefixedName;
     Log(("VBOXSF: MRxCloseSrvOpen: Remaining name = %.*ls, Len = %d\n",
-         RemainingName->Length / sizeof(WCHAR), RemainingName->Buffer, RemainingName->Length));
+         pRemainingName->Length / sizeof(WCHAR), pRemainingName->Buffer, pRemainingName->Length));
+#endif
 
     if (!pVBoxFobx)
@@ -827 +838 @@
     }
 
-    /* Close file */
+    /*
+     * Remove file or directory if delete action is pending and the this is the last open handle.
+     */
+    NTSTATUS Status = STATUS_SUCCESS;
+    if (capFcb->FcbState & FCB_STATE_DELETE_ON_CLOSE)
+    {
+        Log(("VBOXSF: MRxCloseSrvOpen: Delete on close. Open count = %d\n",
+             capFcb->OpenCount));
+
+        if (capFcb->OpenCount == 0)
+            Status = vbsfNtRemove(RxContext);
+    }
+
+    /*
+     * Close file if we still have a handle to it.
+     */
     if (pVBoxFobx->hFile != SHFL_HANDLE_NIL)
         vbsfNtCloseFileHandle(pNetRootExtension, pVBoxFobx, VBoxMRxGetFcbExtension(capFcb));
 
-    if (capFcb->FcbState & FCB_STATE_DELETE_ON_CLOSE)
-    {
-        Log(("VBOXSF: MRxCloseSrvOpen: Delete on close. Open count = %d\n",
-             capFcb->OpenCount));
-
-        /* Remove file or directory if delete action is pending. */
-        if (capFcb->OpenCount == 0)
-            Status = vbsfNtRemove(RxContext);
-    }
-
     return Status;
 }
@@ -846 +862 @@
 /**
  * Worker for vbsfNtSetBasicInfo and VBoxMRxCloseSrvOpen.
+ *
+ * Only called by vbsfNtSetBasicInfo if there is exactly one open handle.  And
+ * VBoxMRxCloseSrvOpen calls it when the last handle is being closed.
  */
 NTSTATUS vbsfNtRemove(IN PRX_CONTEXT RxContext)
 {
-    NTSTATUS Status = STATUS_SUCCESS;
-
     RxCaptureFcb;
     RxCaptureFobx;
-
     PMRX_VBOX_NETROOT_EXTENSION pNetRootExtension = VBoxMRxGetNetRootExtension(capFcb->pNetRoot);
-    PMRX_VBOX_FOBX pVBoxFobx = VBoxMRxGetFileObjectExtension(capFobx);
-
-    PUNICODE_STRING RemainingName = GET_ALREADY_PREFIXED_NAME_FROM_CONTEXT(RxContext);
-
-    int vrc;
-    PSHFLSTRING ParsedPath = NULL;
+    PMRX_VBOX_FOBX              pVBoxFobx         = VBoxMRxGetFileObjectExtension(capFobx);
+    PUNICODE_STRING             pRemainingName    = GET_ALREADY_PREFIXED_NAME_FROM_CONTEXT(RxContext);
+    uint16_t                    cwcRemainingName  = pRemainingName->Length / sizeof(WCHAR);
 
     Log(("VBOXSF: vbsfNtRemove: Delete %.*ls. open count = %d\n",
-         RemainingName->Length / sizeof(WCHAR), RemainingName->Buffer, capFcb->OpenCount));
-
-    /* Close file first if not already done. */
+         cwcRemainingName, pRemainingName->Buffer, capFcb->OpenCount));
+    Assert(RxIsFcbAcquiredExclusive(capFcb));
+
+    /*
+     * We allocate a single request buffer for the closing and deletion to save time.
+     */
+    AssertCompile(sizeof(VBOXSFCLOSEREQ) <= sizeof(VBOXSFREMOVEREQ));
+    AssertReturn((cwcRemainingName + 1) * sizeof(RTUTF16) < _64K, STATUS_NAME_TOO_LONG);
+    size_t cbReq = RT_UOFFSETOF(VBOXSFREMOVEREQ, StrPath.String) + (cwcRemainingName + 1) * sizeof(RTUTF16);
+    union MyCloseAndRemoveReq
+    {
+        VBOXSFCLOSEREQ  Close;
+        VBOXSFREMOVEREQ Remove;
+    } *pReq = (union MyCloseAndRemoveReq *)VbglR0PhysHeapAlloc((uint32_t)cbReq);
+    if (pReq)
+        RT_ZERO(*pReq);
+    else
+        return STATUS_INSUFFICIENT_RESOURCES;
+/** @todo Create a function that combines closing and removing since that is
+ * usually what NT guests will be doing.  Should in theory speed up deletion by 33%. */
+
+    /*
+     * Close file first if not already done.  We dont use vbsfNtCloseFileHandle here
+     * as we got our own request buffer and have no need to update any file info.
+     */
     if (pVBoxFobx->hFile != SHFL_HANDLE_NIL)
-        vbsfNtCloseFileHandle(pNetRootExtension, pVBoxFobx, VBoxMRxGetFcbExtension(capFcb));
-
-    Log(("VBOXSF: vbsfNtRemove: RemainingName->Length %d\n", RemainingName->Length));
-    Status = vbsfNtShflStringFromUnicodeAlloc(&ParsedPath, RemainingName->Buffer, RemainingName->Length);
-    if (Status != STATUS_SUCCESS)
-        return Status;
-
-    /* Call host. */
-    vrc = VbglR0SfRemove(&g_SfClient, &pNetRootExtension->map,
-                         ParsedPath,
-                         pVBoxFobx->Info.Attr.fMode & RTFS_DOS_DIRECTORY ? SHFL_REMOVE_DIR : SHFL_REMOVE_FILE);
-
-    if (ParsedPath)
-        vbsfNtFreeNonPagedMem(ParsedPath);
-
-    if (vrc == VINF_SUCCESS)
+    {
+        int vrcClose = VbglR0SfHostReqClose(pNetRootExtension->map.root, &pReq->Close, pVBoxFobx->hFile);
+        pVBoxFobx->hFile = SHFL_HANDLE_NIL;
+        if (RT_FAILURE(vrcClose))
+            Log(("VBOXSF: vbsfNtRemove: Closing the handle failed! vrcClose %Rrc, hFile %#RX64 (probably)\n",
+                 vrcClose, pReq->Close.Parms.u64Handle.u.value64));
+    }
+
+    /*
+     * Try remove the file.  The path to the file/whatever string is
+     * embedded in the request buffer, so we have to assemble it ourselves here.
+     */
+    uint16_t cwcToCopy = pRemainingName->Length / sizeof(WCHAR);
+    AssertMsg(cwcToCopy == cwcRemainingName, ("%#x, was %#x; FCB exclusivity: %d\n", cwcToCopy, cwcRemainingName, RxIsFcbAcquiredExclusive(capFcb)));
+    if (cwcToCopy <= cwcRemainingName)
+    { /* Extremely likely... */ }
+    else
+    {
+        VbglR0PhysHeapFree(pReq);
+        AssertLogRelMsgFailed(("File scheduled for removal was renamed?!?: %#x from %#x; FCB exclusivity: %d\n",
+                               cwcToCopy, cwcRemainingName, RxIsFcbAcquiredExclusive(capFcb)));
+        cwcRemainingName = cwcToCopy;
+        AssertReturn((cwcRemainingName + 1) * sizeof(RTUTF16) < _64K, STATUS_NAME_TOO_LONG);
+        cbReq = RT_UOFFSETOF(VBOXSFREMOVEREQ, StrPath.String) + (cwcRemainingName + 1) * sizeof(RTUTF16);
+        pReq = (union MyCloseAndRemoveReq *)VbglR0PhysHeapAlloc((uint32_t)cbReq);
+        if (pReq)
+            RT_ZERO(*pReq);
+        else
+            return STATUS_INSUFFICIENT_RESOURCES;
+        AssertMsgReturnStmt(cwcToCopy == pRemainingName->Length / sizeof(WCHAR),
+                            ("%#x, now %#x;\n", cwcToCopy == pRemainingName->Length / sizeof(WCHAR)),
+                            VbglR0PhysHeapFree(pReq), STATUS_INTERNAL_ERROR);
+    }
+
+    memcpy(&pReq->Remove.StrPath.String, pRemainingName->Buffer, cwcToCopy * sizeof(RTUTF16));
+    pReq->Remove.StrPath.String.utf16[cwcToCopy] = '\0';
+    pReq->Remove.StrPath.u16Length = cwcToCopy * 2;
+    pReq->Remove.StrPath.u16Size   = cwcToCopy * 2 + (uint16_t)sizeof(RTUTF16);
+    int vrc = VbglR0SfHostReqRemove(pNetRootExtension->map.root, &pReq->Remove,
+                                    pVBoxFobx->Info.Attr.fMode & RTFS_DOS_DIRECTORY ? SHFL_REMOVE_DIR : SHFL_REMOVE_FILE);
+    NTSTATUS Status;
+    if (RT_SUCCESS(vrc))
+    {
         SetFlag(capFobx->pSrvOpen->Flags, SRVOPEN_FLAG_FILE_DELETED);
-
-    Status = vbsfNtVBoxStatusToNt(vrc);
-    if (vrc != VINF_SUCCESS)
+        vbsfNtCleanupFcbxTimestampRefsOnClose(pVBoxFobx, VBoxMRxGetFcbExtension(capFcb));
+        Status = STATUS_SUCCESS;
+    }
+    else
+    {
         Log(("VBOXSF: vbsfNtRemove: VbglR0SfRemove failed with %Rrc\n", vrc));
-
-    Log(("VBOXSF: vbsfNtRemove: Returned 0x%08X\n", Status));
+        Status = vbsfNtVBoxStatusToNt(vrc);
+    }
+    VbglR0PhysHeapFree(pReq);
+
+    Log(("VBOXSF: vbsfNtRemove: Returned %#010X (%Rrc)\n", Status, vrc));
     return Status;
 }