[cgutman] 52723: [AFD] - Fix a nasty datagram corruption bug that would result in an uninitialized buffer data being returned instead of packet data if the client read buffer was smaller than the d...
Author: cgutman Date: Mon Jul 18 04:21:40 2011 New Revision: 52723
URL: http://svn.reactos.org/svn/reactos?rev=52723&view=rev Log: [AFD] - Fix a nasty datagram corruption bug that would result in an uninitialized buffer data being returned instead of packet data if the client read buffer was smaller than the datagram received - Fix broken user-mode send datagram IRP completion code which didn't set the completion status - Implement disabling/enabling event select triggers
Modified: trunk/reactos/drivers/network/afd/afd/listen.c trunk/reactos/drivers/network/afd/afd/read.c trunk/reactos/drivers/network/afd/afd/select.c trunk/reactos/drivers/network/afd/afd/write.c trunk/reactos/drivers/network/afd/include/afd.h
Modified: trunk/reactos/drivers/network/afd/afd/listen.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/lis... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/listen.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/listen.c [iso-8859-1] Mon Jul 18 04:21:40 2011 @@ -367,13 +367,16 @@ AFD_DbgPrint(MID_TRACE,("Completed a wait for accept\n"));
ExFreePool( PendingConnObj ); - - if( !IsListEmpty( &FCB->PendingConnections ) ) { - FCB->PollState |= AFD_EVENT_ACCEPT; - FCB->PollStatus[FD_ACCEPT_BIT] = STATUS_SUCCESS; + + FCB->EventSelectDisabled &= ~AFD_EVENT_ACCEPT; + + if( !IsListEmpty( &FCB->PendingConnections ) ) + { + FCB->PollState |= AFD_EVENT_ACCEPT; + FCB->PollStatus[FD_ACCEPT_BIT] = STATUS_SUCCESS; PollReeval( FCB->DeviceExt, FCB->FileObject ); - } else - FCB->PollState &= ~AFD_EVENT_ACCEPT; + } else + FCB->PollState &= ~AFD_EVENT_ACCEPT;
SocketStateUnlock( FCB ); return Status;
Modified: trunk/reactos/drivers/network/afd/afd/read.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/rea... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/read.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/read.c [iso-8859-1] Mon Jul 18 04:21:40 2011 @@ -408,9 +408,19 @@
*TotalBytesCopied = BytesToCopy; } - - Status = Irp->IoStatus.Status = STATUS_SUCCESS; - Irp->IoStatus.Information = BytesToCopy; + + if (*TotalBytesCopied == DatagramRecv->Len) + { + /* We copied the whole datagram */ + Status = Irp->IoStatus.Status = STATUS_SUCCESS; + } + else + { + /* We only copied part of the datagram */ + Status = Irp->IoStatus.Status = STATUS_BUFFER_OVERFLOW; + } + + Irp->IoStatus.Information = *TotalBytesCopied;
if (!(RecvReq->TdiFlags & TDI_RECEIVE_PEEK)) { @@ -464,62 +474,46 @@ Irp, 0 ); }
+ FCB->EventSelectDisabled &= ~AFD_EVENT_RECEIVE; + if( FCB->Flags & AFD_ENDPOINT_CONNECTIONLESS ) { - if( !IsListEmpty( &FCB->DatagramList ) ) { - ListEntry = RemoveHeadList( &FCB->DatagramList ); - DatagramRecv = CONTAINING_RECORD - ( ListEntry, AFD_STORED_DATAGRAM, ListEntry ); - if( DatagramRecv->Len > RecvReq->BufferArray[0].len && - !(RecvReq->TdiFlags & TDI_RECEIVE_PARTIAL) ) { - InsertHeadList( &FCB->DatagramList, - &DatagramRecv->ListEntry ); - Status = Irp->IoStatus.Status = STATUS_BUFFER_TOO_SMALL; - Irp->IoStatus.Information = DatagramRecv->Len; - - if( !IsListEmpty( &FCB->DatagramList ) ) { - FCB->PollState |= AFD_EVENT_RECEIVE; - FCB->PollStatus[FD_READ_BIT] = STATUS_SUCCESS; - PollReeval( FCB->DeviceExt, FCB->FileObject ); - } else - FCB->PollState &= ~AFD_EVENT_RECEIVE; - - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, FALSE ); - - AFD_DbgPrint(MIN_TRACE,("Partial datagram not read\n")); - - return UnlockAndMaybeComplete - ( FCB, Status, Irp, Irp->IoStatus.Information ); - } else { - Status = SatisfyPacketRecvRequest - ( FCB, Irp, DatagramRecv, - (PUINT)&Irp->IoStatus.Information ); - - if (RecvReq->TdiFlags & TDI_RECEIVE_PEEK) - { - InsertHeadList(&FCB->DatagramList, - &DatagramRecv->ListEntry); - } - - if( !IsListEmpty( &FCB->DatagramList ) ) { - FCB->PollState |= AFD_EVENT_RECEIVE; - FCB->PollStatus[FD_READ_BIT] = STATUS_SUCCESS; - PollReeval( FCB->DeviceExt, FCB->FileObject ); - } else - FCB->PollState &= ~AFD_EVENT_RECEIVE; - - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, FALSE ); - - return UnlockAndMaybeComplete - ( FCB, Status, Irp, Irp->IoStatus.Information ); + if (!IsListEmpty(&FCB->DatagramList)) + { + ListEntry = RemoveHeadList(&FCB->DatagramList); + DatagramRecv = CONTAINING_RECORD(ListEntry, AFD_STORED_DATAGRAM, ListEntry); + Status = SatisfyPacketRecvRequest(FCB, Irp, DatagramRecv, + (PUINT)&Irp->IoStatus.Information); + + if (RecvReq->TdiFlags & TDI_RECEIVE_PEEK) + { + InsertHeadList(&FCB->DatagramList, + &DatagramRecv->ListEntry); } - } else if( (RecvReq->AfdFlags & AFD_IMMEDIATE) || (FCB->NonBlocking) ) { + + if (IsListEmpty(&FCB->DatagramList)) + { + FCB->PollState |= AFD_EVENT_RECEIVE; + FCB->PollStatus[FD_READ_BIT] = STATUS_SUCCESS; + PollReeval( FCB->DeviceExt, FCB->FileObject ); + } + else + FCB->PollState &= ~AFD_EVENT_RECEIVE; + + UnlockBuffers(RecvReq->BufferArray, RecvReq->BufferCount, FALSE); + + return UnlockAndMaybeComplete(FCB, Status, Irp, Irp->IoStatus.Information); + } + else if( (RecvReq->AfdFlags & AFD_IMMEDIATE) || (FCB->NonBlocking) ) + { AFD_DbgPrint(MID_TRACE,("Nonblocking\n")); Status = STATUS_CANT_WAIT; FCB->PollState &= ~AFD_EVENT_RECEIVE; UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, FALSE ); return UnlockAndMaybeComplete( FCB, Status, Irp, 0 ); - } else { + } + else + { FCB->PollState &= ~AFD_EVENT_RECEIVE; return LeaveIrpUntilLater( FCB, Irp, FUNCTION_RECV ); } @@ -652,34 +646,26 @@ AFD_DbgPrint(MID_TRACE,("RecvReq: %x, DatagramRecv: %x\n", RecvReq, DatagramRecv));
- if( DatagramRecv->Len > RecvReq->BufferArray[0].len && - !(RecvReq->TdiFlags & TDI_RECEIVE_PARTIAL) ) { - InsertHeadList( &FCB->DatagramList, - &DatagramRecv->ListEntry ); - Status = NextIrp->IoStatus.Status = STATUS_BUFFER_TOO_SMALL; - NextIrp->IoStatus.Information = DatagramRecv->Len; - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, CheckUnlockExtraBuffers(FCB, NextIrpSp) ); - if ( NextIrp->MdlAddress ) UnlockRequest( NextIrp, IoGetCurrentIrpStackLocation( NextIrp ) ); - (void)IoSetCancelRoutine(NextIrp, NULL); - AFD_DbgPrint(MIN_TRACE,("Partial datagram failed\n")); - IoCompleteRequest( NextIrp, IO_NETWORK_INCREMENT ); - } else { - AFD_DbgPrint(MID_TRACE,("Satisfying\n")); - Status = SatisfyPacketRecvRequest - ( FCB, NextIrp, DatagramRecv, - (PUINT)&NextIrp->IoStatus.Information ); - if (RecvReq->TdiFlags & TDI_RECEIVE_PEEK) - { - InsertHeadList(&FCB->DatagramList, - &DatagramRecv->ListEntry); - } - AFD_DbgPrint(MID_TRACE,("Unlocking\n")); - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, CheckUnlockExtraBuffers(FCB, NextIrpSp) ); - if ( NextIrp->MdlAddress ) UnlockRequest( NextIrp, IoGetCurrentIrpStackLocation( NextIrp ) ); - AFD_DbgPrint(MID_TRACE,("Completing\n")); - (void)IoSetCancelRoutine(NextIrp, NULL); - IoCompleteRequest( NextIrp, IO_NETWORK_INCREMENT ); - } + AFD_DbgPrint(MID_TRACE,("Satisfying\n")); + Status = SatisfyPacketRecvRequest + ( FCB, NextIrp, DatagramRecv, + (PUINT)&NextIrp->IoStatus.Information ); + + if (RecvReq->TdiFlags & TDI_RECEIVE_PEEK) + { + InsertHeadList(&FCB->DatagramList, + &DatagramRecv->ListEntry); + } + + AFD_DbgPrint(MID_TRACE,("Unlocking\n")); + UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, CheckUnlockExtraBuffers(FCB, NextIrpSp) ); + if ( NextIrp->MdlAddress ) UnlockRequest( NextIrp, IoGetCurrentIrpStackLocation( NextIrp ) ); + + AFD_DbgPrint(MID_TRACE,("Completing\n")); + (void)IoSetCancelRoutine(NextIrp, NULL); + NextIrp->IoStatus.Status = Status; + + IoCompleteRequest( NextIrp, IO_NETWORK_INCREMENT ); }
if( !IsListEmpty( &FCB->DatagramList ) && IsListEmpty(&FCB->PendingIrpList[FUNCTION_RECV]) ) { @@ -747,61 +733,45 @@ ( FCB, STATUS_ACCESS_VIOLATION, Irp, 0 ); }
- if( !IsListEmpty( &FCB->DatagramList ) ) { - ListEntry = RemoveHeadList( &FCB->DatagramList ); - DatagramRecv = CONTAINING_RECORD - ( ListEntry, AFD_STORED_DATAGRAM, ListEntry ); - if( DatagramRecv->Len > RecvReq->BufferArray[0].len && - !(RecvReq->TdiFlags & TDI_RECEIVE_PARTIAL) ) { - InsertHeadList( &FCB->DatagramList, - &DatagramRecv->ListEntry ); - Status = Irp->IoStatus.Status = STATUS_BUFFER_TOO_SMALL; - Irp->IoStatus.Information = DatagramRecv->Len; - - if( !IsListEmpty( &FCB->DatagramList ) ) { - FCB->PollState |= AFD_EVENT_RECEIVE; - FCB->PollStatus[FD_READ_BIT] = STATUS_SUCCESS; - PollReeval( FCB->DeviceExt, FCB->FileObject ); - } else - FCB->PollState &= ~AFD_EVENT_RECEIVE; - - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, TRUE ); - - AFD_DbgPrint(MIN_TRACE,("Partial datagram failed\n")); - - return UnlockAndMaybeComplete - ( FCB, Status, Irp, Irp->IoStatus.Information ); - } else { - Status = SatisfyPacketRecvRequest - ( FCB, Irp, DatagramRecv, - (PUINT)&Irp->IoStatus.Information ); - - if (RecvReq->TdiFlags & TDI_RECEIVE_PEEK) - { - InsertHeadList(&FCB->DatagramList, - &DatagramRecv->ListEntry); - } - - if( !IsListEmpty( &FCB->DatagramList ) ) { - FCB->PollState |= AFD_EVENT_RECEIVE; - FCB->PollStatus[FD_READ_BIT] = STATUS_SUCCESS; - PollReeval( FCB->DeviceExt, FCB->FileObject ); - } else - FCB->PollState &= ~AFD_EVENT_RECEIVE; - - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, TRUE ); - - return UnlockAndMaybeComplete - ( FCB, Status, Irp, Irp->IoStatus.Information ); - } - } else if( (RecvReq->AfdFlags & AFD_IMMEDIATE) || (FCB->NonBlocking) ) { - AFD_DbgPrint(MID_TRACE,("Nonblocking\n")); - Status = STATUS_CANT_WAIT; - FCB->PollState &= ~AFD_EVENT_RECEIVE; - UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, TRUE ); - return UnlockAndMaybeComplete( FCB, Status, Irp, 0 ); - } else { - FCB->PollState &= ~AFD_EVENT_RECEIVE; - return LeaveIrpUntilLater( FCB, Irp, FUNCTION_RECV ); - } -} + FCB->EventSelectDisabled &= ~AFD_EVENT_RECEIVE; + + if (!IsListEmpty(&FCB->DatagramList)) + { + ListEntry = RemoveHeadList(&FCB->DatagramList); + DatagramRecv = CONTAINING_RECORD(ListEntry, AFD_STORED_DATAGRAM, ListEntry); + Status = SatisfyPacketRecvRequest(FCB, Irp, DatagramRecv, + (PUINT)&Irp->IoStatus.Information); + + if (RecvReq->TdiFlags & TDI_RECEIVE_PEEK) + { + InsertHeadList(&FCB->DatagramList, + &DatagramRecv->ListEntry); + } + + if (IsListEmpty(&FCB->DatagramList)) + { + FCB->PollState |= AFD_EVENT_RECEIVE; + FCB->PollStatus[FD_READ_BIT] = STATUS_SUCCESS; + PollReeval( FCB->DeviceExt, FCB->FileObject ); + } + else + FCB->PollState &= ~AFD_EVENT_RECEIVE; + + UnlockBuffers(RecvReq->BufferArray, RecvReq->BufferCount, FALSE); + + return UnlockAndMaybeComplete(FCB, Status, Irp, Irp->IoStatus.Information); + } + else if( (RecvReq->AfdFlags & AFD_IMMEDIATE) || (FCB->NonBlocking) ) + { + AFD_DbgPrint(MID_TRACE,("Nonblocking\n")); + Status = STATUS_CANT_WAIT; + FCB->PollState &= ~AFD_EVENT_RECEIVE; + UnlockBuffers( RecvReq->BufferArray, RecvReq->BufferCount, FALSE ); + return UnlockAndMaybeComplete( FCB, Status, Irp, 0 ); + } + else + { + FCB->PollState &= ~AFD_EVENT_RECEIVE; + return LeaveIrpUntilLater( FCB, Irp, FUNCTION_RECV ); + } +}
Modified: trunk/reactos/drivers/network/afd/afd/select.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/sel... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/select.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/select.c [iso-8859-1] Mon Jul 18 04:21:40 2011 @@ -305,8 +305,15 @@ Status = STATUS_SUCCESS; }
- if( FCB->EventSelect && (FCB->PollState & FCB->EventSelectTriggers) ) { + if((FCB->EventSelect) && + (FCB->PollState & (FCB->EventSelectTriggers & ~FCB->EventSelectDisabled))) + { AFD_DbgPrint(MID_TRACE,("Setting event %x\n", FCB->EventSelect)); + + /* Disable the events that triggered the select until the reenabling function is called */ + FCB->EventSelectDisabled |= (FCB->PollState & (FCB->EventSelectTriggers & ~FCB->EventSelectDisabled)); + + /* Set the application's event */ KeSetEvent( FCB->EventSelect, IO_NETWORK_INCREMENT, FALSE ); }
@@ -408,8 +415,15 @@
KeReleaseSpinLock( &DeviceExt->Lock, OldIrql );
- if( FCB->EventSelect && (FCB->PollState & FCB->EventSelectTriggers) ) { + if((FCB->EventSelect) && + (FCB->PollState & (FCB->EventSelectTriggers & ~FCB->EventSelectDisabled))) + { AFD_DbgPrint(MID_TRACE,("Setting event %x\n", FCB->EventSelect)); + + /* Disable the events that triggered the select until the reenabling function is called */ + FCB->EventSelectDisabled |= (FCB->PollState & (FCB->EventSelectTriggers & ~FCB->EventSelectDisabled)); + + /* Set the application's event */ KeSetEvent( FCB->EventSelect, IO_NETWORK_INCREMENT, FALSE ); }
Modified: trunk/reactos/drivers/network/afd/afd/write.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/wri... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/write.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/write.c [iso-8859-1] Mon Jul 18 04:21:40 2011 @@ -322,6 +322,7 @@ Status = TdiBuildConnectionInfo( &TargetAddress, FCB->RemoteAddress );
if( NT_SUCCESS(Status) ) { + FCB->EventSelectDisabled &= ~AFD_EVENT_SEND; FCB->PollState &= ~AFD_EVENT_SEND;
Status = QueueUserModeIrp(FCB, Irp, FUNCTION_SEND); @@ -438,6 +439,8 @@ SpaceAvail -= SendReq->BufferArray[i].len; }
+ FCB->EventSelectDisabled &= ~AFD_EVENT_SEND; + if( TotalBytesCopied == 0 ) { AFD_DbgPrint(MID_TRACE,("Empty send\n")); UnlockBuffers( SendReq->BufferArray, SendReq->BufferCount, FALSE ); @@ -558,6 +561,7 @@ /* Check the size of the Address given ... */
if( NT_SUCCESS(Status) ) { + FCB->EventSelectDisabled &= ~AFD_EVENT_RECEIVE; FCB->PollState &= ~AFD_EVENT_SEND;
Status = QueueUserModeIrp(FCB, Irp, FUNCTION_SEND);
Modified: trunk/reactos/drivers/network/afd/include/afd.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/include... ============================================================================== --- trunk/reactos/drivers/network/afd/include/afd.h [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/include/afd.h [iso-8859-1] Mon Jul 18 04:21:40 2011 @@ -193,6 +193,7 @@ KMUTEX Mutex; PKEVENT EventSelect; DWORD EventSelectTriggers; + DWORD EventSelectDisabled; UNICODE_STRING TdiDeviceName; PVOID Context; DWORD PollState;
-
cgutman＠svn.reactos.org