From df340872f70218cee5e092612194b21249207188 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Sat, 5 Dec 2015 23:00:33 -0800 Subject: [PATCH] sys: FspFsvolDeviceInsertContext: can no longer fail because of low memory --- src/sys/create.c | 82 ++++++++++++++++++++++++++++-------------------- src/sys/device.c | 27 ++++++++-------- src/sys/driver.h | 14 ++++++++- 3 files changed, 74 insertions(+), 49 deletions(-) diff --git a/src/sys/create.c b/src/sys/create.c index cf2b3539..7db87e97 100644 --- a/src/sys/create.c +++ b/src/sys/create.c @@ -346,6 +346,20 @@ VOID FspFsvolCreateComplete( { FSP_ENTER_IOC(PAGED_CODE()); + /* + * NOTE: + * In the following we may have to close the just opened file object. But we must + * never close a file we just created, because this will leave an orphan file on + * the disk. + * + * Files may have to be closed if a security access or share access check fails. In + * both those cases we were opening an existing file and so it is safe to close it. + * + * Because of how IRP_MJ_CREATE works in Windows, it is difficult to improve on this + * scheme without introducing a 2-phase Create protocol, which is undesirable as it + * means two trips into user-mode for a single Create request. + */ + PDEVICE_OBJECT DeviceObject = IrpSp->DeviceObject; FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(DeviceObject); FSP_FSVRT_DEVICE_EXTENSION *FsvrtDeviceExtension = @@ -400,11 +414,11 @@ VOID FspFsvolCreateComplete( goto reparse_fail; } ExFreePool(FileObject->FileName.Buffer); - FileObject->FileName.Length = 0; FileObject->FileName.MaximumLength = ReparseFileName.Length; FileObject->FileName.Buffer = Buffer; - RtlCopyUnicodeString(&FileObject->FileName, &ReparseFileName); } + FileObject->FileName.Length = 0; + RtlCopyUnicodeString(&FileObject->FileName, &ReparseFileName); } else if (IO_REMOUNT == Response->IoStatus.Information) @@ -471,43 +485,43 @@ VOID FspFsvolCreateComplete( ExAcquireResourceExclusiveLite(&FsvolDeviceExtension->Base.Resource, TRUE); try { - /* attempt to insert the newly created FsContext into our generic table */ + /* insert the newly created FsContext into our generic table */ FsContext = FspFsvolDeviceInsertContext(DeviceObject, - FsContext->UserContext, FsContext, &Inserted); - if (0 == FsContext) - Result = STATUS_INSUFFICIENT_RESOURCES; + FsContext->UserContext, FsContext, &FsContext->ElementStorage, &Inserted); + ASSERT(0 != FsContext); + + /* share access check */ + if (Inserted) + { + /* + * This is a newly created FsContext. Set its share access and + * increment its open count. There is no need to acquire the + * FsContext's Resource (because it is newly created). + */ + IoSetShareAccess(AccessState->PreviouslyGrantedAccess, + ShareAccess, FileObject, &FsContext->ShareAccess); + FspFileContextOpen(FsContext); + Result = STATUS_SUCCESS; + } else { - /* share access check */ - if (Inserted) - { - /* - * This is a newly created FsContext. Set its share access and - * increment its open count. There is no need to acquire the - * FsContext's Resource (because it is newly created). - */ - IoSetShareAccess(AccessState->PreviouslyGrantedAccess, - ShareAccess, FileObject, &FsContext->ShareAccess); - FspFileContextOpen(FsContext); - Result = STATUS_SUCCESS; - } + /* + * This is an existing FsContext. We must acquire its Resource and + * check if there is a delete pending and the share access. Only if + * both tests succeed we increment the open count and report success. + */ + ExAcquireResourceExclusiveLite(FsContext->Header.Resource, TRUE); + if (FsContext->DeletePending) + Result = STATUS_DELETE_PENDING; else + Result = IoCheckShareAccess(AccessState->PreviouslyGrantedAccess, + ShareAccess, FileObject, &FsContext->ShareAccess, TRUE); + if (NT_SUCCESS(Result)) { - /* - * This is an existing FsContext. We must acquire its Resource and - * check if there is a delete pending and the share access. Only if - * both tests succeed we increment the open count and report success. - */ - ExAcquireResourceExclusiveLite(FsContext->Header.Resource, TRUE); - if (FsContext->DeletePending) - Result = STATUS_DELETE_PENDING; - else - Result = IoCheckShareAccess(AccessState->PreviouslyGrantedAccess, - ShareAccess, FileObject, &FsContext->ShareAccess, TRUE); - if (NT_SUCCESS(Result)) - FspFileContextOpen(FsContext); - ExReleaseResourceLite(FsContext->Header.Resource); + FspFileContextRetain(FsContext); + FspFileContextOpen(FsContext); } + ExReleaseResourceLite(FsContext->Header.Resource); } } finally @@ -515,7 +529,7 @@ VOID FspFsvolCreateComplete( ExReleaseResourceLite(&FsvolDeviceExtension->Base.Resource); } - /* did we fail to insert? */ + /* did we fail our share access checks? */ if (!NT_SUCCESS(Result)) { ASSERT(!Inserted); diff --git a/src/sys/device.c b/src/sys/device.c index 5f032751..71246536 100644 --- a/src/sys/device.c +++ b/src/sys/device.c @@ -26,7 +26,7 @@ VOID FspFsctlDeviceVolumeCreated(PDEVICE_OBJECT DeviceObject); VOID FspFsctlDeviceVolumeDeleted(PDEVICE_OBJECT DeviceObject); PVOID FspFsvolDeviceLookupContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier); PVOID FspFsvolDeviceInsertContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier, PVOID Context, - PBOOLEAN PInserted); + FSP_DEVICE_GENERIC_TABLE_ELEMENT *ElementStorage, PBOOLEAN PInserted); VOID FspFsvolDeviceDeleteContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier, PBOOLEAN PDeleted); static RTL_AVL_COMPARE_ROUTINE FspFsvolDeviceCompareElement; @@ -61,12 +61,6 @@ VOID FspDeviceDeleteAll(VOID); #pragma alloc_text(PAGE, FspDeviceDeleteAll) #endif -typedef struct -{ - UINT64 Identifier; - PVOID Context; -} FSP_DEVICE_GENERIC_TABLE_ELEMENT; - NTSTATUS FspDeviceCreateSecure(UINT32 Kind, ULONG ExtraSize, PUNICODE_STRING DeviceName, DEVICE_TYPE DeviceType, PUNICODE_STRING DeviceSddl, LPCGUID DeviceClassGuid, @@ -236,7 +230,7 @@ static VOID FspFsvolDeviceFini(PDEVICE_OBJECT DeviceObject) * Enumerate and delete all entries in the GenericTable. * There is no need to protect accesses to the table as we are in the device destructor. */ - FSP_DEVICE_GENERIC_TABLE_ELEMENT *Element; + FSP_DEVICE_GENERIC_TABLE_ELEMENT_DATA *Element; while (0 != (Element = RtlGetElementGenericTableAvl(&FsvolDeviceExtension->GenericTable, 0))) RtlDeleteElementGenericTableAvl(&FsvolDeviceExtension->GenericTable, &Element->Identifier); @@ -318,7 +312,7 @@ PVOID FspFsvolDeviceLookupContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier ASSERT(FspFsvolDeviceExtensionKind == FsvolDeviceExtension->Base.Kind); ASSERT(ExIsResourceAcquiredExclusiveLite(&FsvolDeviceExtension->Base.Resource)); - FSP_DEVICE_GENERIC_TABLE_ELEMENT *Result; + FSP_DEVICE_GENERIC_TABLE_ELEMENT_DATA *Result; Result = RtlLookupElementGenericTableAvl(&FsvolDeviceExtension->GenericTable, &Identifier); @@ -326,7 +320,7 @@ PVOID FspFsvolDeviceLookupContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier } PVOID FspFsvolDeviceInsertContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier, PVOID Context, - PBOOLEAN PInserted) + FSP_DEVICE_GENERIC_TABLE_ELEMENT *ElementStorage, PBOOLEAN PInserted) { PAGED_CODE(); @@ -334,12 +328,14 @@ PVOID FspFsvolDeviceInsertContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier ASSERT(FspFsvolDeviceExtensionKind == FsvolDeviceExtension->Base.Kind); ASSERT(ExIsResourceAcquiredExclusiveLite(&FsvolDeviceExtension->Base.Resource)); - FSP_DEVICE_GENERIC_TABLE_ELEMENT *Result, Element = { 0 }; + FSP_DEVICE_GENERIC_TABLE_ELEMENT_DATA *Result, Element = { 0 }; Element.Identifier = Identifier; Element.Context = Context; + FsvolDeviceExtension->GenericTableElementStorage = ElementStorage; Result = RtlInsertElementGenericTableAvl(&FsvolDeviceExtension->GenericTable, &Element, sizeof Element, PInserted); + FsvolDeviceExtension->GenericTableElementStorage = 0; return 0 != Result ? Result->Context : 0; } @@ -380,15 +376,18 @@ static PVOID NTAPI FspFsvolDeviceAllocateElement( { PAGED_CODE(); - return ExAllocatePoolWithTag(PagedPool, ByteSize, FSP_TAG); + FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = + CONTAINING_RECORD(Table, FSP_FSVOL_DEVICE_EXTENSION, GenericTable); + + ASSERT(sizeof(FSP_DEVICE_GENERIC_TABLE_ELEMENT) == ByteSize); + + return FsvolDeviceExtension->GenericTableElementStorage; } static VOID NTAPI FspFsvolDeviceFreeElement( PRTL_AVL_TABLE Table, PVOID Buffer) { PAGED_CODE(); - - ExFreePoolWithTag(Buffer, FSP_TAG); } NTSTATUS FspDeviceCopyList( diff --git a/src/sys/driver.h b/src/sys/driver.h index 0f2c39e2..aeee17c4 100644 --- a/src/sys/driver.h +++ b/src/sys/driver.h @@ -305,7 +305,18 @@ typedef struct FSP_DEVICE_EXTENSION Base; PDEVICE_OBJECT FsvrtDeviceObject; RTL_AVL_TABLE GenericTable; + PVOID GenericTableElementStorage; } FSP_FSVOL_DEVICE_EXTENSION; +typedef struct +{ + UINT64 Identifier; + PVOID Context; +} FSP_DEVICE_GENERIC_TABLE_ELEMENT_DATA; +typedef struct +{ + RTL_BALANCED_LINKS Header; + FSP_DEVICE_GENERIC_TABLE_ELEMENT_DATA Data; +} FSP_DEVICE_GENERIC_TABLE_ELEMENT; static inline FSP_DEVICE_EXTENSION *FspDeviceExtension(PDEVICE_OBJECT DeviceObject) { @@ -343,7 +354,7 @@ VOID FspFsctlDeviceVolumeCreated(PDEVICE_OBJECT DeviceObject); VOID FspFsctlDeviceVolumeDeleted(PDEVICE_OBJECT DeviceObject); PVOID FspFsvolDeviceLookupContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier); PVOID FspFsvolDeviceInsertContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier, PVOID Context, - PBOOLEAN PInserted); + FSP_DEVICE_GENERIC_TABLE_ELEMENT *ElementStorage, PBOOLEAN PInserted); VOID FspFsvolDeviceDeleteContext(PDEVICE_OBJECT DeviceObject, UINT64 Identifier, PBOOLEAN PDeleted); NTSTATUS FspDeviceCopyList( @@ -370,6 +381,7 @@ typedef struct SHARE_ACCESS ShareAccess; BOOLEAN DeletePending; /* read-only after creation */ + FSP_DEVICE_GENERIC_TABLE_ELEMENT ElementStorage; UINT64 UserContext; UNICODE_STRING FileName; WCHAR FileNameBuf[];