Intravision/reactos
1
0
Fork 0
mirror of https://github.com/reactos/reactos.git synced 2024-06-28 00:41:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

[AFD]

Browse source 
- Fix one of the worst bugs in AFD of all time. Our AFD code operated under the assumption that none of the input parameters would change once we called LockRequest. This assumption is completely false. The only guarantee that made was that the pages never disappeared from us, not they they couldn't be modified. There are frequent cases where the user-mode buffer was modified from underneath us (WSPRecv allocates the struct on stack which makes it invalid during overlapped operations that complete later). When this happened, we would bugcheck when we tried to unlock the buffers since we accessed this in a member of the struct the caller passed us.
- I've fixed this by adding a parameter to LockRequest which specifies whether the buffer should be copied back when it is unlocked.
- This bug has been around for ages and I was never able to figure out why we just freed garbage sometimes. Now that the ws2_32_winetest exposed it reliably, I was finally able to fix it.

svn path=/trunk/; revision=57175
This commit is contained in:
Cameron Gutman 2012-08-27 04:16:28 +00:00
parent 55be1f528f
commit 6c0d0cbab0
11 changed files with 70 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
reactos/drivers/network/afd/afd/bind.c
View file

@ -81,7 +81,7 @@ AfdBindSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
AFD_DbgPrint(MID_TRACE,("Called\n"));
if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp );
if( !(BindReq = LockRequest( Irp, IrpSp )) )
if( !(BindReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete( FCB, STATUS_NO_MEMORY,
Irp, 0 );

10
reactos/drivers/network/afd/afd/connect.c
View file

@ -44,7 +44,7 @@ AfdSetConnectOptions(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PVOID ConnectOptions = LockRequest(Irp, IrpSp);
PVOID ConnectOptions = LockRequest(Irp, IrpSp, FALSE);
UINT ConnectOptionsSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -80,7 +80,7 @@ AfdSetConnectOptionsSize(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PUINT ConnectOptionsSize = LockRequest(Irp, IrpSp);
PUINT ConnectOptionsSize = LockRequest(Irp, IrpSp, FALSE);
UINT BufferSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -144,7 +144,7 @@ AfdSetConnectData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PVOID ConnectData = LockRequest(Irp, IrpSp);
PVOID ConnectData = LockRequest(Irp, IrpSp, FALSE);
UINT ConnectDataSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -179,7 +179,7 @@ AfdSetConnectDataSize(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PUINT ConnectDataSize = LockRequest(Irp, IrpSp);
PUINT ConnectDataSize = LockRequest(Irp, IrpSp, FALSE);
UINT BufferSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -406,7 +406,7 @@ AfdStreamSocketConnect(PDEVICE_OBJECT DeviceObject, PIRP Irp,
AFD_DbgPrint(MID_TRACE,("Called on %x\n", FCB));
if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp );
if( !(ConnectReq = LockRequest( Irp, IrpSp )) )
if( !(ConnectReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete( FCB, STATUS_NO_MEMORY, Irp,
0 );

2
reactos/drivers/network/afd/afd/context.c
View file

@ -60,7 +60,7 @@ AfdSetContext( PDEVICE_OBJECT DeviceObject, PIRP Irp,
PIO_STACK_LOCATION IrpSp ) {
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PVOID Context = LockRequest(Irp, IrpSp);
PVOID Context = LockRequest(Irp, IrpSp, FALSE);
if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp );

4
reactos/drivers/network/afd/afd/info.c
View file

@ -13,7 +13,7 @@ NTSTATUS NTAPI
AfdGetInfo( PDEVICE_OBJECT DeviceObject, PIRP Irp,
PIO_STACK_LOCATION IrpSp ) {
NTSTATUS Status = STATUS_SUCCESS;
PAFD_INFO InfoReq = LockRequest(Irp, IrpSp);
PAFD_INFO InfoReq = LockRequest(Irp, IrpSp, TRUE);
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PLIST_ENTRY CurrentEntry;
@ -99,7 +99,7 @@ NTSTATUS NTAPI
AfdSetInfo( PDEVICE_OBJECT DeviceObject, PIRP Irp,
PIO_STACK_LOCATION IrpSp ) {
NTSTATUS Status = STATUS_SUCCESS;
PAFD_INFO InfoReq = LockRequest(Irp, IrpSp);
PAFD_INFO InfoReq = LockRequest(Irp, IrpSp, FALSE);
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PCHAR NewBuffer;

2
reactos/drivers/network/afd/afd/listen.c
View file

@ -217,7 +217,7 @@ NTSTATUS AfdListenSocket( PDEVICE_OBJECT DeviceObject, PIRP Irp,
if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp );
if( !(ListenReq = LockRequest( Irp, IrpSp )) )
if( !(ListenReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete( FCB, STATUS_NO_MEMORY, Irp,
0 );

48
reactos/drivers/network/afd/afd/lock.c
View file

@ -12,12 +12,13 @@
PVOID GetLockedData(PIRP Irp, PIO_STACK_LOCATION IrpSp)
{
ASSERT(Irp->MdlAddress);
ASSERT(Irp->Tail.Overlay.DriverContext[0]);
return MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
return Irp->Tail.Overlay.DriverContext[0];
}
/* Lock a method_neither request so it'll be available from DISPATCH_LEVEL */
PVOID LockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp ) {
PVOID LockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp, BOOLEAN Output ) {
BOOLEAN LockFailed = FALSE;
ASSERT(!Irp->MdlAddress);
@ -84,12 +85,55 @@ PVOID LockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp ) {
return NULL;
}
/* The mapped address goes in index 1 */
Irp->Tail.Overlay.DriverContext[1] = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
if (!Irp->Tail.Overlay.DriverContext[1])
{
AFD_DbgPrint(MIN_TRACE,("Failed to get mapped address\n"));
MmUnlockPages(Irp->MdlAddress);
IoFreeMdl( Irp->MdlAddress );
Irp->MdlAddress = NULL;
return NULL;
}
/* The allocated address goes in index 0 */
Irp->Tail.Overlay.DriverContext[0] = ExAllocatePool(NonPagedPool, MmGetMdlByteCount(Irp->MdlAddress));
if (!Irp->Tail.Overlay.DriverContext[0])
{
AFD_DbgPrint(MIN_TRACE,("Failed to allocate memory\n"));
MmUnlockPages(Irp->MdlAddress);
IoFreeMdl( Irp->MdlAddress );
Irp->MdlAddress = NULL;
return NULL;
}
RtlCopyMemory(Irp->Tail.Overlay.DriverContext[0],
Irp->Tail.Overlay.DriverContext[1],
MmGetMdlByteCount(Irp->MdlAddress));
/* If we don't want a copy back, we zero the mapped address pointer */
if (!Output)
{
Irp->Tail.Overlay.DriverContext[1] = NULL;
}
return GetLockedData(Irp, IrpSp);
}
VOID UnlockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp )
{
ASSERT(Irp->MdlAddress);
ASSERT(Irp->Tail.Overlay.DriverContext[0]);
/* Check if we need to copy stuff back */
if (Irp->Tail.Overlay.DriverContext[1] != NULL)
{
RtlCopyMemory(Irp->Tail.Overlay.DriverContext[1],
Irp->Tail.Overlay.DriverContext[0],
MmGetMdlByteCount(Irp->MdlAddress));
}
ExFreePool(Irp->Tail.Overlay.DriverContext[0]);
MmUnlockPages( Irp->MdlAddress );
IoFreeMdl( Irp->MdlAddress );
Irp->MdlAddress = NULL;

12
reactos/drivers/network/afd/afd/main.c
View file

@ -72,7 +72,7 @@ AfdSetDisconnectOptions(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PVOID DisconnectOptions = LockRequest(Irp, IrpSp);
PVOID DisconnectOptions = LockRequest(Irp, IrpSp, FALSE);
UINT DisconnectOptionsSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -108,7 +108,7 @@ AfdSetDisconnectOptionsSize(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PUINT DisconnectOptionsSize = LockRequest(Irp, IrpSp);
PUINT DisconnectOptionsSize = LockRequest(Irp, IrpSp, FALSE);
UINT BufferSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -172,7 +172,7 @@ AfdSetDisconnectData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PVOID DisconnectData = LockRequest(Irp, IrpSp);
PVOID DisconnectData = LockRequest(Irp, IrpSp, FALSE);
UINT DisconnectDataSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -208,7 +208,7 @@ AfdSetDisconnectDataSize(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PUINT DisconnectDataSize = LockRequest(Irp, IrpSp);
PUINT DisconnectDataSize = LockRequest(Irp, IrpSp, FALSE);
UINT BufferSize = IrpSp->Parameters.DeviceIoControl.InputBufferLength;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -244,7 +244,7 @@ AfdGetTdiHandles(PDEVICE_OBJECT DeviceObject, PIRP Irp,
{
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_FCB FCB = FileObject->FsContext;
PULONG HandleFlags = LockRequest(Irp, IrpSp);
PULONG HandleFlags = LockRequest(Irp, IrpSp, TRUE);
PAFD_TDI_HANDLE_DATA HandleData = Irp->UserBuffer;
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
@ -680,7 +680,7 @@ AfdDisconnect(PDEVICE_OBJECT DeviceObject, PIRP Irp,
if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp);
if (!(DisReq = LockRequest(Irp, IrpSp)))
if (!(DisReq = LockRequest(Irp, IrpSp, FALSE)))
return UnlockAndMaybeComplete( FCB, STATUS_NO_MEMORY,
Irp, 0 );

4
reactos/drivers/network/afd/afd/read.c
View file

@ -445,7 +445,7 @@ AfdConnectedSocketReadData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
Irp, 0 );
}
if( !(RecvReq = LockRequest( Irp, IrpSp )) )
if( !(RecvReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete( FCB, STATUS_NO_MEMORY,
Irp, 0 );
@ -715,7 +715,7 @@ AfdPacketSocketReadData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
return UnlockAndMaybeComplete(FCB, STATUS_FILE_CLOSED, Irp, 0);
}
if( !(RecvReq = LockRequest( Irp, IrpSp )) )
if( !(RecvReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete(FCB, STATUS_NO_MEMORY, Irp, 0);
AFD_DbgPrint(MID_TRACE,("Recv flags %x\n", RecvReq->AfdFlags));

4
reactos/drivers/network/afd/afd/select.c
View file

@ -259,7 +259,7 @@ AfdEventSelect( PDEVICE_OBJECT DeviceObject, PIRP Irp,
PFILE_OBJECT FileObject = IrpSp->FileObject;
NTSTATUS Status = STATUS_NO_MEMORY;
PAFD_EVENT_SELECT_INFO EventSelectInfo =
(PAFD_EVENT_SELECT_INFO)LockRequest( Irp, IrpSp );
(PAFD_EVENT_SELECT_INFO)LockRequest( Irp, IrpSp, FALSE );
PAFD_FCB FCB = FileObject->FsContext;
if( !SocketAcquireStateLock( FCB ) ) {
@ -322,7 +322,7 @@ AfdEnumEvents( PDEVICE_OBJECT DeviceObject, PIRP Irp,
PIO_STACK_LOCATION IrpSp ) {
PFILE_OBJECT FileObject = IrpSp->FileObject;
PAFD_ENUM_NETWORK_EVENTS_INFO EnumReq =
(PAFD_ENUM_NETWORK_EVENTS_INFO)LockRequest( Irp, IrpSp );
(PAFD_ENUM_NETWORK_EVENTS_INFO)LockRequest( Irp, IrpSp, TRUE );
PAFD_FCB FCB = FileObject->FsContext;
AFD_DbgPrint(MID_TRACE,("Called (FCB %x)\n", FCB));

6
reactos/drivers/network/afd/afd/write.c
View file

@ -299,7 +299,7 @@ AfdConnectedSocketWriteData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
0 );
}
if( !(SendReq = LockRequest( Irp, IrpSp )) )
if( !(SendReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete( FCB, STATUS_NO_MEMORY, Irp, 0 );
/* Must lock buffers before handing off user data */
@ -369,7 +369,7 @@ AfdConnectedSocketWriteData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
return UnlockAndMaybeComplete(FCB, STATUS_FILE_CLOSED, Irp, 0);
}
if( !(SendReq = LockRequest( Irp, IrpSp )) )
if( !(SendReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete
( FCB, STATUS_NO_MEMORY, Irp, TotalBytesCopied );
@ -516,7 +516,7 @@ AfdPacketSocketWriteData(PDEVICE_OBJECT DeviceObject, PIRP Irp,
return UnlockAndMaybeComplete(FCB, STATUS_FILE_CLOSED, Irp, 0);
}
if( !(SendReq = LockRequest( Irp, IrpSp )) )
if( !(SendReq = LockRequest( Irp, IrpSp, FALSE )) )
return UnlockAndMaybeComplete(FCB, STATUS_NO_MEMORY, Irp, 0);
if (FCB->State == SOCKET_STATE_CREATED)

2
reactos/drivers/network/afd/include/afd.h
View file

@ -308,7 +308,7 @@ VOID SocketStateUnlock( PAFD_FCB FCB );
NTSTATUS LostSocket( PIRP Irp );
PAFD_HANDLE LockHandles( PAFD_HANDLE HandleArray, UINT HandleCount );
VOID UnlockHandles( PAFD_HANDLE HandleArray, UINT HandleCount );
PVOID LockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp );
PVOID LockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp, BOOLEAN Output );
VOID UnlockRequest( PIRP Irp, PIO_STACK_LOCATION IrpSp );
PVOID GetLockedData( PIRP Irp, PIO_STACK_LOCATION IrpSp );
NTSTATUS LeaveIrpUntilLater( PAFD_FCB FCB, PIRP Irp, UINT Function );