|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [win-pv-devel] [PATCH] Release lower bus interface before passing removal IRP to PDO
> -----Original Message----- > From: Paul Durrant [mailto:pdurrant@xxxxxxxxx] > Sent: 27 February 2015 18:10 > To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx > Cc: Paul Durrant > Subject: [PATCH] Release lower bus interface before passing removal IRP to > PDO > > If the PDO is surprised removed then the FDO tries to retain its > reference to the PDO's bus interface byond its destruction. This will > cause an assertion failure in checked builds of XENBUS and may cause > memory corruption. Actually, that comment is wrong. I'll re-post. Paul > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> > --- > src/xenvif/fdo.c | 279 ++++++++++++++++++++++++++++++------------------ > ------- > 1 file changed, 151 insertions(+), 128 deletions(-) > > diff --git a/src/xenvif/fdo.c b/src/xenvif/fdo.c > index 2f2e6bd..d2ce513 100644 > --- a/src/xenvif/fdo.c > +++ b/src/xenvif/fdo.c > @@ -75,7 +75,7 @@ struct _XENVIF_FDO { > PDEVICE_OBJECT LowerDeviceObject; > PDEVICE_OBJECT PhysicalDeviceObject; > DEVICE_CAPABILITIES LowerDeviceCapabilities; > - BUS_INTERFACE_STANDARD LowerBusInterface; > + PBUS_INTERFACE_STANDARD LowerBusInterface; > ULONG Usage[DeviceUsageTypeDumpFile + 1]; > BOOLEAN NotDisableable; > > @@ -216,6 +216,108 @@ FdoGetPhysicalDeviceObject( > return __FdoGetPhysicalDeviceObject(Fdo); > } > > +static FORCEINLINE NTSTATUS > +__FdoAcquireLowerBusInterface( > + IN PXENVIF_FDO Fdo > + ) > +{ > + PBUS_INTERFACE_STANDARD BusInterface; > + KEVENT Event; > + IO_STATUS_BLOCK StatusBlock; > + PIRP Irp; > + PIO_STACK_LOCATION StackLocation; > + NTSTATUS status; > + > + ASSERT3U(KeGetCurrentIrql(), ==, PASSIVE_LEVEL); > + > + BusInterface = __FdoAllocate(sizeof (BUS_INTERFACE_STANDARD)); > + > + status = STATUS_NO_MEMORY; > + if (BusInterface == NULL) > + goto fail1; > + > + KeInitializeEvent(&Event, NotificationEvent, FALSE); > + RtlZeroMemory(&StatusBlock, sizeof(IO_STATUS_BLOCK)); > + > + Irp = IoBuildSynchronousFsdRequest(IRP_MJ_PNP, > + Fdo->LowerDeviceObject, > + NULL, > + 0, > + NULL, > + &Event, > + &StatusBlock); > + > + status = STATUS_UNSUCCESSFUL; > + if (Irp == NULL) > + goto fail2; > + > + StackLocation = IoGetNextIrpStackLocation(Irp); > + StackLocation->MinorFunction = IRP_MN_QUERY_INTERFACE; > + > + StackLocation->Parameters.QueryInterface.InterfaceType = > &GUID_BUS_INTERFACE_STANDARD; > + StackLocation->Parameters.QueryInterface.Size = sizeof > (BUS_INTERFACE_STANDARD); > + StackLocation->Parameters.QueryInterface.Version = 1; > + StackLocation->Parameters.QueryInterface.Interface = > (PINTERFACE)BusInterface; > + > + Irp->IoStatus.Status = STATUS_NOT_SUPPORTED; > + > + status = IoCallDriver(Fdo->LowerDeviceObject, Irp); > + if (status == STATUS_PENDING) { > + (VOID) KeWaitForSingleObject(&Event, > + Executive, > + KernelMode, > + FALSE, > + NULL); > + status = StatusBlock.Status; > + } > + > + if (!NT_SUCCESS(status)) > + goto fail3; > + > + status = STATUS_INVALID_PARAMETER; > + if (BusInterface->Version != 1) > + goto fail4; > + > + Fdo->LowerBusInterface = BusInterface; > + > + return STATUS_SUCCESS; > + > +fail4: > + Error("fail4\n"); > + > +fail3: > + Error("fail3\n"); > + > +fail2: > + Error("fail2\n"); > + > + __FdoFree(BusInterface); > + > +fail1: > + Error("fail1 (%08x)\n", status); > + > + return status; > +} > + > +static FORCEINLINE VOID > +__FdoReleaseLowerBusInterface( > + IN PXENVIF_FDO Fdo > + ) > +{ > + PBUS_INTERFACE_STANDARD BusInterface; > + > + BusInterface = Fdo->LowerBusInterface; > + > + if (BusInterface == NULL) > + return; > + > + Fdo->LowerBusInterface = NULL; > + > + BusInterface->InterfaceDereference(BusInterface->Context); > + > + __FdoFree(BusInterface); > +} > + > PDMA_ADAPTER > FdoGetDmaAdapter( > IN PXENVIF_FDO Fdo, > @@ -223,13 +325,14 @@ FdoGetDmaAdapter( > OUT PULONG NumberOfMapRegisters > ) > { > - PBUS_INTERFACE_STANDARD LowerBusInterface; > + PBUS_INTERFACE_STANDARD BusInterface; > > - LowerBusInterface = &Fdo->LowerBusInterface; > + BusInterface = Fdo->LowerBusInterface; > + ASSERT(BusInterface != NULL); > > - return LowerBusInterface->GetDmaAdapter(LowerBusInterface- > >Context, > - DeviceDescriptor, > - NumberOfMapRegisters); > + return BusInterface->GetDmaAdapter(BusInterface->Context, > + DeviceDescriptor, > + NumberOfMapRegisters); > } > > BOOLEAN > @@ -241,55 +344,58 @@ FdoTranslateBusAddress( > OUT PPHYSICAL_ADDRESS TranslatedAddress > ) > { > - PBUS_INTERFACE_STANDARD LowerBusInterface; > + PBUS_INTERFACE_STANDARD BusInterface; > > - LowerBusInterface = &Fdo->LowerBusInterface; > + BusInterface = Fdo->LowerBusInterface; > + ASSERT(BusInterface != NULL); > > - return LowerBusInterface->TranslateBusAddress(LowerBusInterface- > >Context, > - BusAddress, > - Length, > - AddressSpace, > - TranslatedAddress); > + return BusInterface->TranslateBusAddress(BusInterface->Context, > + BusAddress, > + Length, > + AddressSpace, > + TranslatedAddress); > } > > ULONG > FdoSetBusData( > - IN PXENVIF_FDO Fdo, > - IN ULONG DataType, > - IN PVOID Buffer, > - IN ULONG Offset, > - IN ULONG Length > + IN PXENVIF_FDO Fdo, > + IN ULONG DataType, > + IN PVOID Buffer, > + IN ULONG Offset, > + IN ULONG Length > ) > { > - PBUS_INTERFACE_STANDARD LowerBusInterface; > + PBUS_INTERFACE_STANDARD BusInterface; > > - LowerBusInterface = &Fdo->LowerBusInterface; > + BusInterface = Fdo->LowerBusInterface; > + ASSERT(BusInterface != NULL); > > - return LowerBusInterface->SetBusData(LowerBusInterface->Context, > - DataType, > - Buffer, > - Offset, > - Length); > + return BusInterface->SetBusData(BusInterface->Context, > + DataType, > + Buffer, > + Offset, > + Length); > } > > ULONG > FdoGetBusData( > - IN PXENVIF_FDO Fdo, > - IN ULONG DataType, > - IN PVOID Buffer, > - IN ULONG Offset, > - IN ULONG Length > + IN PXENVIF_FDO Fdo, > + IN ULONG DataType, > + IN PVOID Buffer, > + IN ULONG Offset, > + IN ULONG Length > ) > { > - PBUS_INTERFACE_STANDARD LowerBusInterface; > + PBUS_INTERFACE_STANDARD BusInterface; > > - LowerBusInterface = &Fdo->LowerBusInterface; > + BusInterface = Fdo->LowerBusInterface; > + ASSERT(BusInterface != NULL); > > - return LowerBusInterface->GetBusData(LowerBusInterface->Context, > - DataType, > - Buffer, > - Offset, > - Length); > + return BusInterface->GetBusData(BusInterface->Context, > + DataType, > + Buffer, > + Offset, > + Length); > } > > static FORCEINLINE VOID > @@ -1427,6 +1533,9 @@ FdoRemoveDevice( > > __FdoSetDevicePnpState(Fdo, Deleted); > > + // We must release our reference before the PDO is destroyed > + __FdoReleaseLowerBusInterface(Fdo); > + > Irp->IoStatus.Status = STATUS_SUCCESS; > > IoSkipCurrentIrpStackLocation(Irp); > @@ -2524,88 +2633,6 @@ FdoDispatch( > return status; > } > > -static FORCEINLINE NTSTATUS > -__FdoAcquireLowerBusInterface( > - IN PXENVIF_FDO Fdo > - ) > -{ > - KEVENT Event; > - IO_STATUS_BLOCK StatusBlock; > - PIRP Irp; > - PIO_STACK_LOCATION StackLocation; > - NTSTATUS status; > - > - ASSERT3U(KeGetCurrentIrql(), ==, PASSIVE_LEVEL); > - > - KeInitializeEvent(&Event, NotificationEvent, FALSE); > - RtlZeroMemory(&StatusBlock, sizeof(IO_STATUS_BLOCK)); > - > - Irp = IoBuildSynchronousFsdRequest(IRP_MJ_PNP, > - Fdo->LowerDeviceObject, > - NULL, > - 0, > - NULL, > - &Event, > - &StatusBlock); > - > - status = STATUS_UNSUCCESSFUL; > - if (Irp == NULL) > - goto fail1; > - > - StackLocation = IoGetNextIrpStackLocation(Irp); > - StackLocation->MinorFunction = IRP_MN_QUERY_INTERFACE; > - > - StackLocation->Parameters.QueryInterface.InterfaceType = > &GUID_BUS_INTERFACE_STANDARD; > - StackLocation->Parameters.QueryInterface.Size = sizeof > (BUS_INTERFACE_STANDARD); > - StackLocation->Parameters.QueryInterface.Version = 1; > - StackLocation->Parameters.QueryInterface.Interface = > (PINTERFACE)&Fdo->LowerBusInterface; > - > - Irp->IoStatus.Status = STATUS_NOT_SUPPORTED; > - > - status = IoCallDriver(Fdo->LowerDeviceObject, Irp); > - if (status == STATUS_PENDING) { > - (VOID) KeWaitForSingleObject(&Event, > - Executive, > - KernelMode, > - FALSE, > - NULL); > - status = StatusBlock.Status; > - } > - > - if (!NT_SUCCESS(status)) > - goto fail2; > - > - status = STATUS_INVALID_PARAMETER; > - if (Fdo->LowerBusInterface.Version != 1) > - goto fail3; > - > - return STATUS_SUCCESS; > - > -fail3: > - Error("fail3\n"); > - > -fail2: > - Error("fail2\n"); > - > -fail1: > - Error("fail1 (%08x)\n", status); > - > - return status; > -} > - > -static FORCEINLINE VOID > -__FdoReleaseLowerBusInterface( > - IN PXENVIF_FDO Fdo > - ) > -{ > - PBUS_INTERFACE_STANDARD BusInterface; > - > - BusInterface = &Fdo->LowerBusInterface; > - BusInterface->InterfaceDereference(BusInterface->Context); > - > - RtlZeroMemory(BusInterface, sizeof (BUS_INTERFACE_STANDARD)); > -} > - > static NTSTATUS > FdoQueryInterface( > IN PXENVIF_FDO Fdo, > @@ -2718,7 +2745,6 @@ FdoCreate( > PDEVICE_OBJECT FunctionDeviceObject; > PXENVIF_DX Dx; > PXENVIF_FDO Fdo; > - PBUS_INTERFACE_STANDARD BusInterface; > USHORT DeviceID; > NTSTATUS status; > > @@ -2765,14 +2791,11 @@ FdoCreate( > if (!NT_SUCCESS(status)) > goto fail5; > > - BusInterface = &Fdo->LowerBusInterface; > - > - status = STATUS_UNSUCCESSFUL; > - if (BusInterface->GetBusData(BusInterface->Context, > - PCI_WHICHSPACE_CONFIG, > - &DeviceID, > - FIELD_OFFSET(PCI_COMMON_HEADER, DeviceID), > - FIELD_SIZE(PCI_COMMON_HEADER, DeviceID)) == > 0) > + if (FdoGetBusData(Fdo, > + PCI_WHICHSPACE_CONFIG, > + &DeviceID, > + FIELD_OFFSET(PCI_COMMON_HEADER, DeviceID), > + FIELD_SIZE(PCI_COMMON_HEADER, DeviceID)) == 0) > goto fail6; > > __FdoSetVendorName(Fdo, DeviceID); > -- > 2.1.1 _______________________________________________ win-pv-devel mailing list win-pv-devel@xxxxxxxxxxxxxxxxxxxx http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |