|
[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 > Sent: 27 February 2015 18:13 > To: 'Paul Durrant'; win-pv-devel@xxxxxxxxxxxxxxxxxxxx > Subject: RE: [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. > No, it's right here. It's wrong in the identically named patch to XENBUS. Paul > 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 |