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/us... ============================================================================== --- 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/us... ============================================================================== --- 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/us... ============================================================================== --- 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;