diff --git a/reactos/drivers/network/afd/afd/listen.c b/reactos/drivers/network/afd/afd/listen.c index c57bb0a9309..ad22f00a8bb 100644 --- a/reactos/drivers/network/afd/afd/listen.c +++ b/reactos/drivers/network/afd/afd/listen.c @@ -140,7 +140,13 @@ static NTSTATUS NTAPI ListenComplete } AFD_DbgPrint(MID_TRACE,("Completing listen request.\n")); - AFD_DbgPrint(MID_TRACE,("IoStatus was %x\n", FCB->ListenIrp.Iosb.Status)); + AFD_DbgPrint(MID_TRACE,("IoStatus was %x\n", Irp->IoStatus.Status)); + + if (Irp->IoStatus.Status != STATUS_SUCCESS) + { + SocketStateUnlock(FCB); + return Irp->IoStatus.Status; + } Qelt = ExAllocatePool( NonPagedPool, sizeof(*Qelt) ); if( !Qelt ) { diff --git a/reactos/drivers/network/afd/afd/lock.c b/reactos/drivers/network/afd/afd/lock.c index 640cf22aae5..55b8fc0f264 100644 --- a/reactos/drivers/network/afd/afd/lock.c +++ b/reactos/drivers/network/afd/afd/lock.c @@ -252,10 +252,38 @@ NTSTATUS LostSocket( PIRP Irp ) { } NTSTATUS LeaveIrpUntilLater( PAFD_FCB FCB, PIRP Irp, UINT Function ) { + NTSTATUS Status; + + /* Add the IRP to the queue in all cases (so AfdCancelHandler will work properly) */ InsertTailList( &FCB->PendingIrpList[Function], - &Irp->Tail.Overlay.ListEntry ); - IoMarkIrpPending(Irp); - (void)IoSetCancelRoutine(Irp, AfdCancelHandler); + &Irp->Tail.Overlay.ListEntry ); + + /* Acquire the cancel spin lock and check the cancel bit */ + IoAcquireCancelSpinLock(&Irp->CancelIrql); + if (!Irp->Cancel) + { + /* We are not cancelled; we're good to go so + * set the cancel routine, release the cancel spin lock, + * mark the IRP as pending, and + * return STATUS_PENDING to the caller + */ + (void)IoSetCancelRoutine(Irp, AfdCancelHandler); + IoReleaseCancelSpinLock(Irp->CancelIrql); + IoMarkIrpPending(Irp); + Status = STATUS_PENDING; + } + else + { + /* We were already cancelled before we were able to register our cancel routine + * so we are to call the cancel routine ourselves right here to cancel the IRP + * (which handles all the stuff we do above) and return STATUS_CANCELLED to the caller + */ + AfdCancelHandler(IoGetCurrentIrpStackLocation(Irp)->DeviceObject, + Irp); + Status = STATUS_CANCELLED; + } + SocketStateUnlock( FCB ); - return STATUS_PENDING; + + return Status; } diff --git a/reactos/drivers/network/afd/afd/main.c b/reactos/drivers/network/afd/afd/main.c index 204c67b6834..0e27f5021b2 100644 --- a/reactos/drivers/network/afd/afd/main.c +++ b/reactos/drivers/network/afd/afd/main.c @@ -762,6 +762,33 @@ AfdDispatch(PDEVICE_OBJECT DeviceObject, PIRP Irp) return (Status); } +VOID +CleanupPendingIrp(PIRP Irp, PIO_STACK_LOCATION IrpSp, PAFD_ACTIVE_POLL Poll) +{ + PAFD_RECV_INFO RecvReq; + PAFD_SEND_INFO SendReq; + PAFD_POLL_INFO PollReq; + + if (IrpSp->Parameters.DeviceIoControl.IoControlCode == IOCTL_AFD_RECV || + IrpSp->MajorFunction == IRP_MJ_READ) + { + RecvReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer; + UnlockBuffers(RecvReq->BufferArray, RecvReq->BufferCount, FALSE); + } + else if (IrpSp->Parameters.DeviceIoControl.IoControlCode == IOCTL_AFD_SEND || + IrpSp->MajorFunction == IRP_MJ_WRITE) + { + SendReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer; + UnlockBuffers(SendReq->BufferArray, SendReq->BufferCount, FALSE); + } + else if (IrpSp->Parameters.DeviceIoControl.IoControlCode == IOCTL_AFD_SELECT) + { + PollReq = Irp->AssociatedIrp.SystemBuffer; + ZeroEvents(PollReq->Handles, PollReq->HandleCount); + SignalSocket(Poll, NULL, PollReq, STATUS_CANCELLED); + } +} + VOID NTAPI AfdCancelHandler(PDEVICE_OBJECT DeviceObject, PIRP Irp) @@ -769,39 +796,46 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject, PIO_STACK_LOCATION IrpSp = IoGetCurrentIrpStackLocation(Irp); PFILE_OBJECT FileObject = IrpSp->FileObject; PAFD_FCB FCB = FileObject->FsContext; - UINT Function; - PAFD_RECV_INFO RecvReq; - PAFD_SEND_INFO SendReq; - PLIST_ENTRY CurrentEntry; + ULONG Function, IoctlCode; PIRP CurrentIrp; + PLIST_ENTRY CurrentEntry; PAFD_DEVICE_EXTENSION DeviceExt = DeviceObject->DeviceExtension; KIRQL OldIrql; PAFD_ACTIVE_POLL Poll; - PAFD_POLL_INFO PollReq; IoReleaseCancelSpinLock(Irp->CancelIrql); - + if (!SocketAcquireStateLock(FCB)) return; + + switch (IrpSp->MajorFunction) + { + case IRP_MJ_DEVICE_CONTROL: + IoctlCode = IrpSp->Parameters.DeviceIoControl.IoControlCode; + break; + + case IRP_MJ_READ: + IoctlCode = IOCTL_AFD_RECV; + break; + + case IRP_MJ_WRITE: + IoctlCode = IOCTL_AFD_SEND; + break; + + default: + ASSERT(FALSE); + SocketStateUnlock(FCB); + return; + } - ASSERT(IrpSp->MajorFunction == IRP_MJ_DEVICE_CONTROL); - - switch (IrpSp->Parameters.DeviceIoControl.IoControlCode) + switch (IoctlCode) { case IOCTL_AFD_RECV: - RecvReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer; - UnlockBuffers(RecvReq->BufferArray, RecvReq->BufferCount, FALSE); - /* Fall through */ - case IOCTL_AFD_RECV_DATAGRAM: Function = FUNCTION_RECV; break; case IOCTL_AFD_SEND: - SendReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer; - UnlockBuffers(SendReq->BufferArray, SendReq->BufferCount, FALSE); - /* Fall through */ - case IOCTL_AFD_SEND_DATAGRAM: Function = FUNCTION_SEND; break; @@ -821,14 +855,13 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject, while (CurrentEntry != &DeviceExt->Polls) { Poll = CONTAINING_RECORD(CurrentEntry, AFD_ACTIVE_POLL, ListEntry); - CurrentIrp = Poll->Irp; - PollReq = CurrentIrp->AssociatedIrp.SystemBuffer; - if (CurrentIrp == Irp) + if (Irp == Poll->Irp) { - ZeroEvents(PollReq->Handles, PollReq->HandleCount); - SignalSocket(Poll, NULL, PollReq, STATUS_CANCELLED); - break; + CleanupPendingIrp(Irp, IrpSp, Poll); + KeReleaseSpinLock(&DeviceExt->Lock, OldIrql); + SocketStateUnlock(FCB); + return; } else { @@ -838,8 +871,9 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject, KeReleaseSpinLock(&DeviceExt->Lock, OldIrql); - /* IRP already completed by SignalSocket */ SocketStateUnlock(FCB); + + DbgPrint("WARNING!!! IRP cancellation race could lead to a process hang! (IOCTL_AFD_SELECT)\n"); return; default: @@ -856,7 +890,9 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject, if (CurrentIrp == Irp) { RemoveEntryList(CurrentEntry); - break; + CleanupPendingIrp(Irp, IrpSp, NULL); + UnlockAndMaybeComplete(FCB, STATUS_CANCELLED, Irp, 0); + return; } else { @@ -864,7 +900,9 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject, } } - UnlockAndMaybeComplete(FCB, STATUS_CANCELLED, Irp, 0); + SocketStateUnlock(FCB); + + DbgPrint("WARNING!!! IRP cancellation race could lead to a process hang! (Function: %d)\n", Function); } static VOID NTAPI diff --git a/reactos/drivers/network/afd/afd/write.c b/reactos/drivers/network/afd/afd/write.c index 7506489be8e..e10b386ba7e 100644 --- a/reactos/drivers/network/afd/afd/write.c +++ b/reactos/drivers/network/afd/afd/write.c @@ -185,9 +185,12 @@ static NTSTATUS NTAPI PacketSocketSendComplete FCB->SendIrp.InFlightRequest = NULL; /* Request is not in flight any longer */ - FCB->PollState |= AFD_EVENT_SEND; - FCB->PollStatus[FD_WRITE_BIT] = STATUS_SUCCESS; - PollReeval( FCB->DeviceExt, FCB->FileObject ); + if (Irp->IoStatus.Status == STATUS_SUCCESS) + { + FCB->PollState |= AFD_EVENT_SEND; + FCB->PollStatus[FD_WRITE_BIT] = STATUS_SUCCESS; + PollReeval( FCB->DeviceExt, FCB->FileObject ); + } if( FCB->State == SOCKET_STATE_CLOSED ) { /* Cleanup our IRP queue because the FCB is being destroyed */