24 Jan
2012
24 Jan
'12
10:28 p.m.
Author: cgutman
Date: Tue Jan 24 22:28:44 2012
New Revision: 55155
URL: http://svn.reactos.org/svn/reactos?rev=55155&view=rev
Log:
[USBSTOR]
- Fix broken IRP error handling and leaking memory
Modified:
branches/usb-bringup-trunk/drivers/usb/usbstor/error.c
branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c
branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/error.c
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/u…
==============================================================================
--- branches/usb-bringup-trunk/drivers/usb/usbstor/error.c [iso-8859-1] (original)
+++ branches/usb-bringup-trunk/drivers/usb/usbstor/error.c [iso-8859-1] Tue Jan 24
22:28:44 2012
@@ -22,14 +22,14 @@
//
// allocate urb
//
- DPRINT1("Allocating URB\n");
+ DPRINT1("Allocating URB\n");
Urb = (PURB)AllocateItem(NonPagedPool, sizeof(struct _URB_PIPE_REQUEST));
if (!Urb)
{
//
// out of memory
//
- DPRINT1("OutofMemory!\n");
+ DPRINT1("OutofMemory!\n");
return STATUS_INSUFFICIENT_RESOURCES;
}
@@ -43,7 +43,7 @@
//
// send the request
//
- DPRINT1("Sending Request DeviceObject %x, Urb %x\n", DeviceObject, Urb);
+ DPRINT1("Sending Request DeviceObject %x, Urb %x\n", DeviceObject, Urb);
Status = USBSTOR_SyncUrbRequest(DeviceObject, Urb);
//
@@ -59,19 +59,19 @@
NTSTATUS
USBSTOR_HandleTransferError(
- PDEVICE_OBJECT DeviceObject,
- PIRP Irp,
- PIRP_CONTEXT Context)
+ PDEVICE_OBJECT DeviceObject,
+ PIRP_CONTEXT Context)
{
- NTSTATUS Status;
- PIO_STACK_LOCATION Stack;
- USBD_PIPE_HANDLE PipeHandle;
- PSCSI_REQUEST_BLOCK Request;
+ NTSTATUS Status;
+ PIO_STACK_LOCATION Stack;
+ USBD_PIPE_HANDLE PipeHandle;
+ PSCSI_REQUEST_BLOCK Request;
+ PCDB pCDB;
- DPRINT1("Entered Handle Transfer Error\n");
- //
- // Determine pipehandle
- //
+ DPRINT1("Entered Handle Transfer Error\n");
+ //
+ // Determine pipehandle
+ //
if (Context->cbw->CommandBlock[0] == SCSIOP_WRITE)
{
//
@@ -87,72 +87,93 @@
PipeHandle =
Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle;
}
- switch (Context->Urb.UrbHeader.Status)
- {
- case USBD_STATUS_STALL_PID:
- {
- //
- // First attempt to reset the pipe
- //
- DPRINT1("Resetting Pipe\n");
- Status = USBSTOR_ResetPipeWithHandle(DeviceObject, PipeHandle);
- if (NT_SUCCESS(Status))
- {
- Status = STATUS_SUCCESS;
- break;
- }
+ switch (Context->Urb.UrbHeader.Status)
+ {
+ case USBD_STATUS_STALL_PID:
+ {
+ //
+ // First attempt to reset the pipe
+ //
+ DPRINT1("Resetting Pipe\n");
+ Status = USBSTOR_ResetPipeWithHandle(DeviceObject, PipeHandle);
+ if (NT_SUCCESS(Status))
+ {
+ Status = STATUS_SUCCESS;
+ break;
+ }
- DPRINT1("Failed to reset pipe %x\n", Status);
+ DPRINT1("Failed to reset pipe %x\n", Status);
- //
- // FIXME: Reset of pipe failed, attempt to reset port
- //
-
- Status = STATUS_UNSUCCESSFUL;
- break;
- }
- //
- // FIXME: Handle more errors
- //
- default:
- {
- DPRINT1("Error not handled\n");
- Status = STATUS_UNSUCCESSFUL;
- }
- }
+ //
+ // FIXME: Reset of pipe failed, attempt to reset port
+ //
+
+ Status = STATUS_UNSUCCESSFUL;
+ break;
+ }
+ //
+ // FIXME: Handle more errors
+ //
+ default:
+ {
+ DPRINT1("Error not handled\n");
+ Status = STATUS_UNSUCCESSFUL;
+ }
+ }
- if (Status != STATUS_SUCCESS)
- {
- Irp->IoStatus.Status = Status;
- Irp->IoStatus.Information = 0;
- }
- else
- {
- Stack = IoGetCurrentIrpStackLocation(Context->Irp);
- //
- // Retry the operation
- //
- Request = (PSCSI_REQUEST_BLOCK)Stack->Parameters.Others.Argument1;
- DPRINT1("Retrying\n");
- Status = USBSTOR_HandleExecuteSCSI(DeviceObject, Context->Irp);
- }
-
- DPRINT1("USBSTOR_HandleTransferError returning with Status %x\n", Status);
- return Status;
+ Stack = IoGetCurrentIrpStackLocation(Context->Irp);
+ Request = (PSCSI_REQUEST_BLOCK)Stack->Parameters.Others.Argument1;
+ pCDB = (PCDB)Request->Cdb;
+ if (Status != STATUS_SUCCESS)
+ {
+ /* Complete the master IRP */
+ Context->Irp->IoStatus.Status = Status;
+ Context->Irp->IoStatus.Information = 0;
+ IoCompleteRequest(Context->Irp, IO_NO_INCREMENT);
+
+ /* Start the next request */
+
USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject,
TRUE);
+ USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject);
+
+ /* Signal the context event */
+ if (Context->Event)
+ KeSetEvent(Context->Event, 0, FALSE);
+
+ /* Cleanup the IRP context */
+ if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY)
+ FreeItem(Context->TransferData);
+ FreeItem(Context->cbw);
+ FreeItem(Context);
+ }
+ else
+ {
+
+ DPRINT1("Retrying\n");
+ Status = USBSTOR_HandleExecuteSCSI(DeviceObject, Context->Irp);
+
+ /* Cleanup the old IRP context */
+ if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY)
+ FreeItem(Context->TransferData);
+ FreeItem(Context->cbw);
+ FreeItem(Context);
+ }
+
+ DPRINT1("USBSTOR_HandleTransferError returning with Status %x\n", Status);
+ return Status;
}
VOID
NTAPI
ErrorHandlerWorkItemRoutine(
- PVOID Context)
+ PVOID Context)
{
- NTSTATUS Status;
- PERRORHANDLER_WORKITEM_DATA WorkItemData = (PERRORHANDLER_WORKITEM_DATA)Context;
-
- Status = USBSTOR_HandleTransferError(WorkItemData->DeviceObject,
WorkItemData->Irp, WorkItemData->Context);
+ NTSTATUS Status;
+ PERRORHANDLER_WORKITEM_DATA WorkItemData = (PERRORHANDLER_WORKITEM_DATA)Context;
+
+ Status = USBSTOR_HandleTransferError(WorkItemData->DeviceObject,
WorkItemData->Context);
- //
- // Free Work Item Data
- //
- ExFreePool(WorkItemData);
+ //
+ // Free Work Item Data
+ //
+ ExFreePool(WorkItemData);
}
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/u…
==============================================================================
--- branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c [iso-8859-1] (original)
+++ branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c [iso-8859-1] Tue Jan 24 22:28:44
2012
@@ -181,18 +181,9 @@
{
DPRINT1("Attempting Error Recovery\n");
//
- // If a Read Capacity Request free TransferBuffer
- //
- if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY)
- {
- FreeItem(Context->TransferData);
- }
-
- //
- // Clean up the rest
- //
- FreeItem(Context->cbw);
- FreeItem(Context);
+ // free the allocated irp
+ //
+ IoFreeIrp(Irp);
//
// Allocate Work Item Data
@@ -213,7 +204,6 @@
ErrorHandlerWorkItemData);
ErrorHandlerWorkItemData->DeviceObject =
Context->FDODeviceExtension->FunctionalDeviceObject;
- ErrorHandlerWorkItemData->Irp = Irp;
ErrorHandlerWorkItemData->Context = Context;
DPRINT1("Queuing WorkItemROutine\n");
ExQueueWorkItem(&ErrorHandlerWorkItemData->WorkQueueItem,
DelayedWorkQueue);
@@ -315,6 +305,10 @@
KeSetEvent(Context->Event, 0, FALSE);
}
+ //
+ // free our allocated irp
+ //
+ IoFreeIrp(Irp);
//
// free context
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/u…
==============================================================================
--- branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h [iso-8859-1] (original)
+++ branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h [iso-8859-1] Tue Jan 24
22:28:44 2012
@@ -278,7 +278,6 @@
typedef struct _ERRORHANDLER_WORKITEM_DATA
{
PDEVICE_OBJECT DeviceObject;
- PIRP Irp;
PIRP_CONTEXT Context;
WORK_QUEUE_ITEM WorkQueueItem;
} ERRORHANDLER_WORKITEM_DATA, *PERRORHANDLER_WORKITEM_DATA;