[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[win-pv-devel] [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.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.