Index: /trunk/src/VBox/Additions/WINNT/SharedFolders/driver/path.cpp =================================================================== --- /trunk/src/VBox/Additions/WINNT/SharedFolders/driver/path.cpp (revision 78464) +++ /trunk/src/VBox/Additions/WINNT/SharedFolders/driver/path.cpp (revision 78465) @@ -695,79 +695,11 @@ /** - * 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,4 +711,82 @@ 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,8 +804,9 @@ } +/** + * @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,13 +816,13 @@ 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,18 +838,23 @@ } - /* 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,48 +862,99 @@ /** * 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; }