From 2f5db208deede6c9f4280da30684bfcc275c2788 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Tue, 24 Jan 2012 22:28:44 +0000 Subject: [PATCH] [USBSTOR] - Fix broken IRP error handling and leaking memory svn path=/branches/usb-bringup-trunk/; revision=55155 --- drivers/usb/usbstor/error.c | 175 +++++++++++++++++++--------------- drivers/usb/usbstor/scsi.c | 18 ++-- drivers/usb/usbstor/usbstor.h | 1 - 3 files changed, 104 insertions(+), 90 deletions(-) diff --git a/drivers/usb/usbstor/error.c b/drivers/usb/usbstor/error.c index 386d2ebc130..80ed2bcc395 100644 --- a/drivers/usb/usbstor/error.c +++ b/drivers/usb/usbstor/error.c @@ -22,14 +22,14 @@ USBSTOR_ResetPipeWithHandle( // // 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 @@ USBSTOR_ResetPipeWithHandle( // // 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 @@ USBSTOR_ResetPipeWithHandle( 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; - - DPRINT1("Entered Handle Transfer Error\n"); - // - // Determine pipehandle - // + NTSTATUS Status; + PIO_STACK_LOCATION Stack; + USBD_PIPE_HANDLE PipeHandle; + PSCSI_REQUEST_BLOCK Request; + PCDB pCDB; + + DPRINT1("Entered Handle Transfer Error\n"); + // + // Determine pipehandle + // if (Context->cbw->CommandBlock[0] == SCSIOP_WRITE) { // @@ -87,72 +87,93 @@ USBSTOR_HandleTransferError( 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; - } - - 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; - } - } - - 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; + 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); + + // + // 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; + } + } + + 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); - - // - // Free Work Item Data - // - ExFreePool(WorkItemData); + NTSTATUS Status; + PERRORHANDLER_WORKITEM_DATA WorkItemData = (PERRORHANDLER_WORKITEM_DATA)Context; + + Status = USBSTOR_HandleTransferError(WorkItemData->DeviceObject, WorkItemData->Context); + + // + // Free Work Item Data + // + ExFreePool(WorkItemData); } diff --git a/drivers/usb/usbstor/scsi.c b/drivers/usb/usbstor/scsi.c index 6db9e1db3eb..3c79700d96c 100644 --- a/drivers/usb/usbstor/scsi.c +++ b/drivers/usb/usbstor/scsi.c @@ -181,18 +181,9 @@ USBSTOR_CSWCompletionRoutine( { DPRINT1("Attempting Error Recovery\n"); // - // If a Read Capacity Request free TransferBuffer + // free the allocated irp // - if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY) - { - FreeItem(Context->TransferData); - } - - // - // Clean up the rest - // - FreeItem(Context->cbw); - FreeItem(Context); + IoFreeIrp(Irp); // // Allocate Work Item Data @@ -213,7 +204,6 @@ USBSTOR_CSWCompletionRoutine( ErrorHandlerWorkItemData); ErrorHandlerWorkItemData->DeviceObject = Context->FDODeviceExtension->FunctionalDeviceObject; - ErrorHandlerWorkItemData->Irp = Irp; ErrorHandlerWorkItemData->Context = Context; DPRINT1("Queuing WorkItemROutine\n"); ExQueueWorkItem(&ErrorHandlerWorkItemData->WorkQueueItem, DelayedWorkQueue); @@ -315,6 +305,10 @@ USBSTOR_CSWCompletionRoutine( KeSetEvent(Context->Event, 0, FALSE); } + // + // free our allocated irp + // + IoFreeIrp(Irp); // // free context diff --git a/drivers/usb/usbstor/usbstor.h b/drivers/usb/usbstor/usbstor.h index 3f3b90743ef..2a043ec7c7b 100644 --- a/drivers/usb/usbstor/usbstor.h +++ b/drivers/usb/usbstor/usbstor.h @@ -278,7 +278,6 @@ typedef struct typedef struct _ERRORHANDLER_WORKITEM_DATA { PDEVICE_OBJECT DeviceObject; - PIRP Irp; PIRP_CONTEXT Context; WORK_QUEUE_ITEM WorkQueueItem; } ERRORHANDLER_WORKITEM_DATA, *PERRORHANDLER_WORKITEM_DATA; -- 2.17.1