From 40c5aa3f5ba3cb16f05e8bed2cacf3175e927fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Sun, 11 Nov 2018 17:17:48 +0100 Subject: [PATCH] [FASTFAT] Improvements for volume dismount + minor bugfixing. - Cache the RootFcb so that its cleanup can be handled separately during dismounting. - Force volume dismount at cleanup if the VCB_DISMOUNT_PENDING flag is set. - Actually dismount a volume if its VCB has been flagged as not good, or if we force dismounting. NOTE: In their *CheckForDismount() function, our 3rd-party FS drivers as well as MS' fastfat, perform a comparison check of the current VCB's VPB ReferenceCount with some sort of "dangling"/"residual" open count. It seems to be related to the fact that the volume root directory as well as auxiliary data stream(s) are still opened, and only these are allowed to be opened at that moment. After analysis it appears that for the ReactOS' fastfat, this number is equal to "3". But this should be investigated more closely... - On dismounting, cleanup and destroy the RootFcb, VolumeFcb and the FATFileObject. Then cleanup the SpareVPB or the IoVPB members, and finish by removing the dismounted volume from the VolumeListEntry and cleaning up the notify synchronization object and the resources. - During dismounting, and on shutdown, flush the volume before resetting its dirty bit. - On shutdown, after volume flushing, try to unmount it without forcing. - Release the VCB resources only when we actually dismount the volume in VfatCheckForDismount(). - Initialize first the notify list and the synchronization object, before sending the FSRTL_VOLUME_MOUNT notification. - If we failed at mounting a volume but its VCB's FATFileObject was already initialized, first call CcUninitializeCacheMap() on it before dereferencing it. - Send FSRTL_VOLUME_LOCK, FSRTL_VOLUME_LOCK_FAILED and FSRTL_VOLUME_UNLOCK notifications during volume locking (and failure) and volume unlocking. - Flush the volume before locking it, and clean its dirty bit if needed. NOTE: In addition to checking for VCB_CLEAR_DIRTY, we also check for the presence of the VCB_IS_DIRTY flag before cleaning up the dirty bit: this allows us to not re-clean the bit if it has been previously cleaned. This is needed for instance in this scenario: - The volume is locked (it gets flushed and the dirty bit is possibly cleared); - The volume then gets formatted with a completely different FS, that possibly clears up the first sector (e.g. BTRFS ignores 1st sector); - The volume is then dismounted: if we didn't check whether VCB_IS_DIRTY was set prior to resetting it, we could attempt clearing it again! But now that the volume's filesystem has been completely changed, we would then try to modify the dirty bit on an erroneous position on disk! That's why it should not be touched in this case during dismounting. - The volume is unlocked (same comment as above), and later can be detected as being BTRFS. [NTOS:IO] Finally remove the dreadful IopParseDevice() hack?! --- drivers/filesystems/fastfat/cleanup.c | 2 +- drivers/filesystems/fastfat/fcb.c | 7 ++ drivers/filesystems/fastfat/fsctl.c | 75 +++++++++++++----- drivers/filesystems/fastfat/misc.c | 101 ++++++++++++++++++++----- drivers/filesystems/fastfat/shutdown.c | 19 +++-- drivers/filesystems/fastfat/vfat.h | 12 +-- ntoskrnl/io/iomgr/file.c | 23 ------ 7 files changed, 163 insertions(+), 76 deletions(-) diff --git a/drivers/filesystems/fastfat/cleanup.c b/drivers/filesystems/fastfat/cleanup.c index 291cd77a2fd..d09df956ca4 100644 --- a/drivers/filesystems/fastfat/cleanup.c +++ b/drivers/filesystems/fastfat/cleanup.c @@ -161,7 +161,7 @@ VfatCleanupFile( #ifdef ENABLE_SWAPOUT if (IsVolume && BooleanFlagOn(DeviceExt->Flags, VCB_DISMOUNT_PENDING)) { - VfatCheckForDismount(DeviceExt, FALSE); + VfatCheckForDismount(DeviceExt, TRUE); } #endif diff --git a/drivers/filesystems/fastfat/fcb.c b/drivers/filesystems/fastfat/fcb.c index ed85ab277c0..33a310c05be 100644 --- a/drivers/filesystems/fastfat/fcb.c +++ b/drivers/filesystems/fastfat/fcb.c @@ -277,6 +277,7 @@ vfatDestroyFCB( #endif FsRtlUninitializeFileLock(&pFCB->FileLock); + if (!vfatFCBIsRoot(pFCB) && !BooleanFlagOn(pFCB->Flags, FCB_IS_FAT) && !BooleanFlagOn(pFCB->Flags, FCB_IS_VOLUME)) { @@ -647,6 +648,8 @@ vfatMakeRootFCB( NTSTATUS Status = STATUS_SUCCESS; UNICODE_STRING NameU = RTL_CONSTANT_STRING(L"\\"); + ASSERT(pVCB->RootFcb == NULL); + FCB = vfatNewFCB(pVCB, &NameU); if (vfatVolumeIsFatX(pVCB)) { @@ -690,6 +693,9 @@ vfatMakeRootFCB( vfatFCBInitializeCacheFromVolume(pVCB, FCB); vfatAddFCBToTable(pVCB, FCB); + /* Cache it */ + pVCB->RootFcb = FCB; + return FCB; } @@ -705,6 +711,7 @@ vfatOpenRootFCB( { FCB = vfatMakeRootFCB(pVCB); } + ASSERT(FCB == pVCB->RootFcb); return FCB; } diff --git a/drivers/filesystems/fastfat/fsctl.c b/drivers/filesystems/fastfat/fsctl.c index 6523487368c..b8f3393d121 100644 --- a/drivers/filesystems/fastfat/fsctl.c +++ b/drivers/filesystems/fastfat/fsctl.c @@ -775,7 +775,7 @@ VfatMount( ReadVolumeLabel(DeviceExt, 0, vfatVolumeIsFatX(DeviceExt), &VolumeLabelU); Vpb->VolumeLabelLength = VolumeLabelU.Length; - /* read dirty bit status */ + /* Read dirty bit status */ Status = GetDirtyStatus(DeviceExt, &Dirty); if (NT_SUCCESS(Status)) { @@ -784,23 +784,29 @@ VfatMount( { /* Mark it dirty now! */ SetDirtyStatus(DeviceExt, TRUE); - VolumeFcb->Flags |= VCB_CLEAR_DIRTY; + SetFlag(VolumeFcb->Flags, VCB_CLEAR_DIRTY); } else { DPRINT1("Mounting a dirty volume\n"); } } + SetFlag(VolumeFcb->Flags, VCB_IS_DIRTY); - VolumeFcb->Flags |= VCB_IS_DIRTY; if (BooleanFlagOn(Vpb->RealDevice->Flags, DO_SYSTEM_BOOT_PARTITION)) { SetFlag(DeviceExt->Flags, VCB_IS_SYS_OR_HAS_PAGE); } - FsRtlNotifyVolumeEvent(DeviceExt->FATFileObject, FSRTL_VOLUME_MOUNT); - FsRtlNotifyInitializeSync(&DeviceExt->NotifySync); + /* Initialize the notify list and synchronization object */ InitializeListHead(&DeviceExt->NotifyList); + FsRtlNotifyInitializeSync(&DeviceExt->NotifySync); + + /* The VCB is OK for usage */ + SetFlag(DeviceExt->Flags, VCB_GOOD); + + /* Send the mount notification */ + FsRtlNotifyVolumeEvent(DeviceExt->FATFileObject, FSRTL_VOLUME_MOUNT); DPRINT("Mount success\n"); @@ -811,7 +817,14 @@ VfatMount( { /* Cleanup */ if (DeviceExt && DeviceExt->FATFileObject) - ObDereferenceObject (DeviceExt->FATFileObject); + { + LARGE_INTEGER Zero = {{0,0}}; + CcUninitializeCacheMap(DeviceExt->FATFileObject, + &Zero, + NULL); + ObDereferenceObject(DeviceExt->FATFileObject); + DeviceExt->FATFileObject = NULL; + } if (DeviceExt && DeviceExt->SpareVPB) ExFreePoolWithTag(DeviceExt->SpareVPB, TAG_VPB); if (DeviceExt && DeviceExt->Statistics) @@ -1081,8 +1094,7 @@ VfatMarkVolumeDirty( { Status = SetDirtyStatus(DeviceExt, TRUE); } - - DeviceExt->VolumeFcb->Flags &= ~VCB_CLEAR_DIRTY; + ClearFlag(DeviceExt->VolumeFcb->Flags, VCB_CLEAR_DIRTY); return Status; } @@ -1125,6 +1137,11 @@ VfatLockOrUnlockVolume( return STATUS_ACCESS_DENIED; } + if (Lock) + { + FsRtlNotifyVolumeEvent(IrpContext->Stack->FileObject, FSRTL_VOLUME_LOCK); + } + /* Deny locking if we're not alone */ if (Lock && DeviceExt->OpenHandleCount != 1) { @@ -1208,6 +1225,8 @@ VfatLockOrUnlockVolume( } } + FsRtlNotifyVolumeEvent(IrpContext->Stack->FileObject, FSRTL_VOLUME_LOCK_FAILED); + return STATUS_ACCESS_DENIED; #if 1 @@ -1225,6 +1244,18 @@ VfatLockOrUnlockVolume( /* Finally, proceed */ if (Lock) { + /* Flush volume & files */ + VfatFlushVolume(DeviceExt, DeviceExt->VolumeFcb); + + /* The volume is now clean */ + if (BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_CLEAR_DIRTY) && + BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_IS_DIRTY)) + { + /* Drop the dirty bit */ + if (NT_SUCCESS(SetDirtyStatus(DeviceExt, FALSE))) + ClearFlag(DeviceExt->VolumeFcb->Flags, VCB_IS_DIRTY); + } + DeviceExt->Flags |= VCB_VOLUME_LOCKED; Vpb->Flags |= VPB_LOCKED; } @@ -1232,6 +1263,8 @@ VfatLockOrUnlockVolume( { DeviceExt->Flags &= ~VCB_VOLUME_LOCKED; Vpb->Flags &= ~VPB_LOCKED; + + FsRtlNotifyVolumeEvent(IrpContext->Stack->FileObject, FSRTL_VOLUME_UNLOCK); } return STATUS_SUCCESS; @@ -1277,25 +1310,33 @@ VfatDismountVolume( ExAcquireResourceExclusiveLite(&DeviceExt->FatResource, TRUE); - /* We're performing a clean shutdown */ - if (BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_CLEAR_DIRTY)) + /* Flush volume & files */ + VfatFlushVolume(DeviceExt, (PVFATFCB)FileObject->FsContext); + + /* The volume is now clean */ + if (BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_CLEAR_DIRTY) && + BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_IS_DIRTY)) { /* Drop the dirty bit */ if (NT_SUCCESS(SetDirtyStatus(DeviceExt, FALSE))) - DeviceExt->VolumeFcb->Flags &= ~VCB_IS_DIRTY; + ClearFlag(DeviceExt->VolumeFcb->Flags, VCB_IS_DIRTY); } - /* Flush volume & files */ - VfatFlushVolume(DeviceExt, (PVFATFCB)FileObject->FsContext); - /* Rebrowse the FCB in order to free them now */ while (!IsListEmpty(&DeviceExt->FcbListHead)) { NextEntry = RemoveTailList(&DeviceExt->FcbListHead); Fcb = CONTAINING_RECORD(NextEntry, VFATFCB, FcbListEntry); vfatDestroyFCB(Fcb); + if (Fcb == DeviceExt->RootFcb) + DeviceExt->RootFcb = NULL; + else if (Fcb == DeviceExt->VolumeFcb) + DeviceExt->VolumeFcb = NULL; } + /* We are uninitializing, the VCB cannot be used anymore */ + ClearFlag(DeviceExt->Flags, VCB_GOOD); + /* Mark we're being dismounted */ DeviceExt->Flags |= VCB_DISMOUNT_PENDING; #ifndef ENABLE_SWAPOUT @@ -1304,12 +1345,6 @@ VfatDismountVolume( ExReleaseResourceLite(&DeviceExt->FatResource); - /* Release a few resources and quit, we're done */ - ExFreePoolWithTag(DeviceExt->Statistics, TAG_STATS); - ExDeleteResourceLite(&DeviceExt->DirResource); - ExDeleteResourceLite(&DeviceExt->FatResource); - ObDereferenceObject(DeviceExt->FATFileObject); - return STATUS_SUCCESS; } diff --git a/drivers/filesystems/fastfat/misc.c b/drivers/filesystems/fastfat/misc.c index ad5f4bcec3e..e7f7385b630 100644 --- a/drivers/filesystems/fastfat/misc.c +++ b/drivers/filesystems/fastfat/misc.c @@ -416,23 +416,43 @@ VfatLockUserBuffer( BOOLEAN VfatCheckForDismount( IN PDEVICE_EXTENSION DeviceExt, - IN BOOLEAN Create) + IN BOOLEAN Force) { KIRQL OldIrql; + ULONG UnCleanCount = 3; // FIXME: or 2 depending on whether we are in IRP_MJ_CREATE; however I think we still leak like 1 or 2 extra refs there... PVPB Vpb; BOOLEAN Delete; - DPRINT1("VfatCheckForDismount(%p, %u)\n", DeviceExt, Create); + DPRINT1("VfatCheckForDismount(%p, %u)\n", DeviceExt, Force); + + /* If the VCB is OK (not under uninitialization) and we don't force dismount, do nothing */ + if (BooleanFlagOn(DeviceExt->Flags, VCB_GOOD) && !Force) + { + return FALSE; + } + +#if 0 // See the other 3rd-party IFSes, or fastfat_new that uses instead a "ResidualOpenCount". + if ((IrpContext->MajorFunction == IRP_MJ_CREATE) && + (IrpContext->RealDevice == Vcb->RealDevice)) + { + UnCleanCount = 3; + } + else + { + UnCleanCount = 2; + } +#endif /* Lock VPB */ IoAcquireVpbSpinLock(&OldIrql); /* Reference it and check if a create is being done */ Vpb = DeviceExt->IoVPB; - if (Vpb->ReferenceCount != Create) + DPRINT1("Vpb->ReferenceCount = %d\n", Vpb->ReferenceCount); + if (Vpb->ReferenceCount != UnCleanCount || DeviceExt->OpenHandleCount != 0) { /* Copy the VPB to our local own to prepare later dismount */ - if (DeviceExt->SpareVPB != NULL) + if (Force && DeviceExt->SpareVPB != NULL) { RtlZeroMemory(DeviceExt->SpareVPB, sizeof(VPB)); DeviceExt->SpareVPB->Type = IO_TYPE_VPB; @@ -443,6 +463,9 @@ VfatCheckForDismount( DeviceExt->IoVPB->RealDevice->Vpb = DeviceExt->SpareVPB; DeviceExt->SpareVPB = NULL; DeviceExt->IoVPB->Flags |= VPB_PERSISTENT; + + /* We are uninitializing, the VCB cannot be used anymore */ + ClearFlag(DeviceExt->Flags, VCB_GOOD); } /* Don't do anything */ @@ -467,34 +490,72 @@ VfatCheckForDismount( /* If we were to delete, delete volume */ if (Delete) { - PVPB DelVpb; + LARGE_INTEGER Zero = {{0,0}}; + + /* We are uninitializing, the VCB cannot be used anymore */ + ClearFlag(DeviceExt->Flags, VCB_GOOD); + + /* Close internal opens */ + if (DeviceExt->RootFcb) + { +#if 0 + CcUninitializeCacheMap(DeviceExt->RootFcb->FileObject, + &Zero, + NULL); + ObDereferenceObject(DeviceExt->RootFcb->FileObject); +#endif + vfatDestroyFCB(DeviceExt->RootFcb); + DeviceExt->RootFcb = NULL; + } + if (DeviceExt->VolumeFcb) + { +#if 0 + CcUninitializeCacheMap(DeviceExt->VolumeFcb->FileObject, + &Zero, + NULL); + ObDereferenceObject(DeviceExt->VolumeFcb->FileObject); +#endif + vfatDestroyFCB(DeviceExt->VolumeFcb); + DeviceExt->VolumeFcb = NULL; + } + if (DeviceExt->FATFileObject) + { + CcUninitializeCacheMap(DeviceExt->FATFileObject, + &Zero, + NULL); + ObDereferenceObject(DeviceExt->FATFileObject); + DeviceExt->FATFileObject = NULL; + } /* If we have a local VPB, we'll have to delete it * but we won't dismount us - something went bad before */ if (DeviceExt->SpareVPB) { - DelVpb = DeviceExt->SpareVPB; + ExFreePool(DeviceExt->SpareVPB); } - /* Otherwise, dismount our device if possible */ - else + /* Otherwise, delete any of the available VPB if its reference count is zero */ + else if (DeviceExt->IoVPB->ReferenceCount == 0) { - if (DeviceExt->IoVPB->ReferenceCount) - { - ObfDereferenceObject(DeviceExt->StorageDevice); - IoDeleteDevice(DeviceExt->VolumeDevice); - return Delete; - } - - DelVpb = DeviceExt->IoVPB; + ExFreePool(DeviceExt->IoVPB); } - /* Delete any of the available VPB and dismount */ - ExFreePool(DelVpb); + /* Remove the volume from the list */ + ExAcquireResourceExclusiveLite(&VfatGlobalData->VolumeListLock, TRUE); + RemoveEntryList(&DeviceExt->VolumeListEntry); + ExReleaseResourceLite(&VfatGlobalData->VolumeListLock); + + /* Uninitialize the notify synchronization object */ + FsRtlNotifyUninitializeSync(&DeviceExt->NotifySync); + + /* Release resources */ + ExFreePoolWithTag(DeviceExt->Statistics, TAG_STATS); + ExDeleteResourceLite(&DeviceExt->DirResource); + ExDeleteResourceLite(&DeviceExt->FatResource); + + /* Dismount our device if possible */ ObfDereferenceObject(DeviceExt->StorageDevice); IoDeleteDevice(DeviceExt->VolumeDevice); - - return Delete; } return Delete; diff --git a/drivers/filesystems/fastfat/shutdown.c b/drivers/filesystems/fastfat/shutdown.c index c9e2bd5431d..75ce0668b05 100644 --- a/drivers/filesystems/fastfat/shutdown.c +++ b/drivers/filesystems/fastfat/shutdown.c @@ -74,15 +74,19 @@ VfatShutdown( ListEntry = ListEntry->Flink; ExAcquireResourceExclusiveLite(&DeviceExt->DirResource, TRUE); - /* It was a clean volume mounted */ - if (DeviceExt->VolumeFcb->Flags & VCB_CLEAR_DIRTY) + + /* Flush volume & files */ + Status = VfatFlushVolume(DeviceExt, DeviceExt->VolumeFcb); + + /* We're performing a clean shutdown */ + if (BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_CLEAR_DIRTY) && + BooleanFlagOn(DeviceExt->VolumeFcb->Flags, VCB_IS_DIRTY)) { - /* So, drop the dirty bit we set */ + /* Drop the dirty bit */ if (NT_SUCCESS(SetDirtyStatus(DeviceExt, FALSE))) - DeviceExt->VolumeFcb->Flags &= ~VCB_IS_DIRTY; + ClearFlag(DeviceExt->VolumeFcb->Flags, VCB_IS_DIRTY); } - Status = VfatFlushVolume(DeviceExt, DeviceExt->VolumeFcb); if (NT_SUCCESS(Status)) { Status = VfatDiskShutDown(DeviceExt); @@ -97,7 +101,10 @@ VfatShutdown( } ExReleaseResourceLite(&DeviceExt->DirResource); - /* FIXME: Unmount the logical volume */ + /* Unmount the logical volume */ +#ifdef ENABLE_SWAPOUT + VfatCheckForDismount(DeviceExt, FALSE); +#endif if (!NT_SUCCESS(Status)) Irp->IoStatus.Status = Status; diff --git a/drivers/filesystems/fastfat/vfat.h b/drivers/filesystems/fastfat/vfat.h index e6c7c73e1d5..e3ba36ae6b8 100644 --- a/drivers/filesystems/fastfat/vfat.h +++ b/drivers/filesystems/fastfat/vfat.h @@ -17,11 +17,7 @@ #endif #define USE_ROS_CC_AND_FS -#if 0 -#ifndef _MSC_VER #define ENABLE_SWAPOUT -#endif -#endif #define ROUND_DOWN(n, align) \ (((ULONG)n) & ~((align) - 1l)) @@ -244,6 +240,9 @@ typedef union _DIR_ENTRY DIR_ENTRY, *PDIR_ENTRY; #define VCB_IS_SYS_OR_HAS_PAGE 0x0008 #define VCB_IS_DIRTY 0x4000 /* Volume is dirty */ #define VCB_CLEAR_DIRTY 0x8000 /* Clean dirty flag at shutdown */ +/* VCB condition state */ +#define VCB_BAD 0x0000 +#define VCB_GOOD 0x0010 typedef struct { @@ -279,7 +278,7 @@ typedef struct _HASHENTRY } HASHENTRY; -typedef struct DEVICE_EXTENSION *PDEVICE_EXTENSION; +typedef struct _DEVICE_EXTENSION *PDEVICE_EXTENSION; typedef NTSTATUS (*PGET_NEXT_CLUSTER)(PDEVICE_EXTENSION,ULONG,PULONG); typedef NTSTATUS (*PFIND_AND_MARK_AVAILABLE_CLUSTER)(PDEVICE_EXTENSION,PULONG); @@ -307,7 +306,7 @@ typedef struct _STATISTICS { UCHAR Pad[((STATISTICS_SIZE_NO_PAD + 0x3f) & ~0x3f) - STATISTICS_SIZE_NO_PAD]; } STATISTICS, *PSTATISTICS; -typedef struct DEVICE_EXTENSION +typedef struct _DEVICE_EXTENSION { ERESOURCE DirResource; ERESOURCE FatResource; @@ -326,6 +325,7 @@ typedef struct DEVICE_EXTENSION BOOLEAN AvailableClustersValid; ULONG Flags; struct _VFATFCB *VolumeFcb; + struct _VFATFCB *RootFcb; PSTATISTICS Statistics; /* Pointers to functions for manipulating FAT. */ diff --git a/ntoskrnl/io/iomgr/file.c b/ntoskrnl/io/iomgr/file.c index d8bb8526065..6b6b2e4abe1 100644 --- a/ntoskrnl/io/iomgr/file.c +++ b/ntoskrnl/io/iomgr/file.c @@ -582,29 +582,6 @@ IopParseDevice(IN PVOID ParseObject, /* Check if we can simply use a dummy file */ UseDummyFile = ((OpenPacket->QueryOnly) || (OpenPacket->DeleteOnly)); -#if 1 - /* FIXME: Small hack still exists, have to check why... - * This is triggered multiple times by usetup and then once per boot. - */ - if (ExpInTextModeSetup && - !(DirectOpen) && - !(RemainingName->Length) && - !(OpenPacket->RelatedFileObject) && - ((wcsstr(CompleteName->Buffer, L"Harddisk")) || - (wcsstr(CompleteName->Buffer, L"Floppy"))) && - !(UseDummyFile)) - { - DPRINT1("Using IopParseDevice() hack. Requested invalid attributes: %lx\n", - DesiredAccess & ~(SYNCHRONIZE | - FILE_READ_ATTRIBUTES | - READ_CONTROL | - ACCESS_SYSTEM_SECURITY | - WRITE_OWNER | - WRITE_DAC)); - DirectOpen = TRUE; - } -#endif - /* Check if this is a direct open */ if (!(RemainingName->Length) && !(OpenPacket->RelatedFileObject) &&