[ros-diffs] [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/li…
==============================================================================
--- 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/re…
==============================================================================
--- 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/se…
==============================================================================
--- 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/wr…
==============================================================================
--- 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/includ…
==============================================================================
--- 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;