From 42e498c4b0a2fc82112bfeb93f5301f8cbb3ab38 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Thu, 13 Aug 2009 23:38:57 +0000 Subject: [PATCH] - Fix a handle leak - Fix a potential NULL pointer dereference if ExAllocatePool fails - Fix a potential NULL pointer dereference that causes AFD to crash when the socket is closed with waiting send IRPs - Fix another NULL pointer dereference if NdisOpenConfiguration fails - Move ASSERT before accessing Status - Add some sanity checks - Most of these were found by Amine Khaldi svn path=/trunk/; revision=42659 --- reactos/drivers/network/afd/afd/select.c | 1 - reactos/drivers/network/afd/afd/tdiconn.c | 3 ++- reactos/drivers/network/afd/afd/write.c | 2 ++ reactos/drivers/network/ndis/ndis/miniport.c | 6 ++++++ reactos/drivers/network/ndis/ndis/misc.c | 4 ++-- reactos/drivers/network/tcpip/tcpip/dispatch.c | 5 +---- reactos/drivers/network/tcpip/tcpip/info.c | 4 ++-- reactos/drivers/network/tcpip/tcpip/ninfo.c | 13 +++++-------- 8 files changed, 20 insertions(+), 18 deletions(-) diff --git a/reactos/drivers/network/afd/afd/select.c b/reactos/drivers/network/afd/afd/select.c index ec2bd92027d..1c045d4059c 100644 --- a/reactos/drivers/network/afd/afd/select.c +++ b/reactos/drivers/network/afd/afd/select.c @@ -207,7 +207,6 @@ AfdSelect( PDEVICE_OBJECT DeviceObject, PIRP Irp, if( (FCB->PollState & AFD_EVENT_CLOSE) || (PollReq->Handles[i].Status & AFD_EVENT_CLOSE) ) { - AFD_HANDLES(PollReq)[i].Handle = 0; PollReq->Handles[i].Events = 0; PollReq->Handles[i].Status = AFD_EVENT_CLOSE; Signalled++; diff --git a/reactos/drivers/network/afd/afd/tdiconn.c b/reactos/drivers/network/afd/afd/tdiconn.c index 1f500ab332f..72418b199d3 100644 --- a/reactos/drivers/network/afd/afd/tdiconn.c +++ b/reactos/drivers/network/afd/afd/tdiconn.c @@ -44,7 +44,8 @@ VOID TaCopyAddressInPlace( PTA_ADDRESS Target, PTA_ADDRESS TaCopyAddress( PTA_ADDRESS Source ) { UINT AddrLen = TaLengthOfAddress( Source ); PVOID Buffer = ExAllocatePool( NonPagedPool, AddrLen ); - RtlCopyMemory( Buffer, Source, AddrLen ); + if (Buffer) + RtlCopyMemory( Buffer, Source, AddrLen ); return Buffer; } diff --git a/reactos/drivers/network/afd/afd/write.c b/reactos/drivers/network/afd/afd/write.c index 78d3b709c19..f163c368152 100644 --- a/reactos/drivers/network/afd/afd/write.c +++ b/reactos/drivers/network/afd/afd/write.c @@ -51,6 +51,8 @@ static NTSTATUS NTAPI SendComplete while( !IsListEmpty( &FCB->PendingIrpList[FUNCTION_SEND] ) ) { NextIrpEntry = RemoveHeadList(&FCB->PendingIrpList[FUNCTION_SEND]); NextIrp = CONTAINING_RECORD(NextIrpEntry, IRP, Tail.Overlay.ListEntry); + NextIrpSp = IoGetCurrentIrpStackLocation( NextIrp ); + SendReq = NextIrpSp->Parameters.DeviceIoControl.Type3InputBuffer; NextIrp->IoStatus.Status = STATUS_FILE_CLOSED; NextIrp->IoStatus.Information = 0; UnlockBuffers(SendReq->BufferArray, SendReq->BufferCount, FALSE); diff --git a/reactos/drivers/network/ndis/ndis/miniport.c b/reactos/drivers/network/ndis/ndis/miniport.c index f24ad39603f..5fa40f0129b 100644 --- a/reactos/drivers/network/ndis/ndis/miniport.c +++ b/reactos/drivers/network/ndis/ndis/miniport.c @@ -1850,6 +1850,12 @@ NdisIPnPStartDevice( */ NdisOpenConfiguration(&NdisStatus, &ConfigHandle, (NDIS_HANDLE)&WrapperContext); + if (NdisStatus != NDIS_STATUS_SUCCESS) + { + NDIS_DbgPrint(MIN_TRACE, ("Failed to open configuration key\n")); + ExInterlockedRemoveEntryList( &Adapter->ListEntry, &AdapterListLock ); + return NdisStatus; + } Size = sizeof(ULONG); Status = IoGetDeviceProperty(Adapter->NdisMiniportBlock.PhysicalDeviceObject, diff --git a/reactos/drivers/network/ndis/ndis/misc.c b/reactos/drivers/network/ndis/ndis/misc.c index 418f6ed0f1c..f8b05a80849 100644 --- a/reactos/drivers/network/ndis/ndis/misc.c +++ b/reactos/drivers/network/ndis/ndis/misc.c @@ -237,11 +237,11 @@ NdisOpenFile( NDIS_DbgPrint(MAX_TRACE, ("Called.\n")); + ASSERT ( Status && FileName ); + *Status = NDIS_STATUS_SUCCESS; FullFileName.Buffer = NULL; - ASSERT ( Status && FileName ); - FullFileName.Length = sizeof(NDIS_FILE_FOLDER); FullFileName.MaximumLength = FileName->MaximumLength + sizeof(NDIS_FILE_FOLDER); FullFileName.Buffer = ExAllocatePool ( NonPagedPool, FullFileName.MaximumLength ); diff --git a/reactos/drivers/network/tcpip/tcpip/dispatch.c b/reactos/drivers/network/tcpip/tcpip/dispatch.c index 0b4e2f5dd30..4b0aa94426f 100644 --- a/reactos/drivers/network/tcpip/tcpip/dispatch.c +++ b/reactos/drivers/network/tcpip/tcpip/dispatch.c @@ -582,10 +582,7 @@ NTSTATUS DispTdiListen( TI_DbgPrint(MIN_TRACE, ("Connection->AddressFile: %x\n", Connection->AddressFile )); - if( Connection->AddressFile ) { - TI_DbgPrint(MIN_TRACE, ("Connection->AddressFile->Listener: %x\n", - Connection->AddressFile->Listener)); - } + ASSERT(Connection->AddressFile); Status = DispPrepareIrpForCancel (TranContext->Handle.ConnectionContext, diff --git a/reactos/drivers/network/tcpip/tcpip/info.c b/reactos/drivers/network/tcpip/tcpip/info.c index 08a4d6d156e..04bdd471be5 100644 --- a/reactos/drivers/network/tcpip/tcpip/info.c +++ b/reactos/drivers/network/tcpip/tcpip/info.c @@ -19,7 +19,7 @@ TDI_STATUS InfoCopyOut( PCHAR DataOut, UINT SizeOut, /* The driver returns success even when it couldn't fit every available * byte. */ - if( RememberedCBSize < SizeOut ) + if( RememberedCBSize < SizeOut || !ClientBuf ) return TDI_SUCCESS; else { CopyBufferToBufferChain( ClientBuf, 0, (PCHAR)DataOut, SizeOut ); @@ -99,7 +99,7 @@ TDI_STATUS InfoTdiQueryListEntities(PNDIS_BUFFER Buffer, TI_DbgPrint(DEBUG_INFO,("BufSize: %d, NeededSize: %d\n", BufSize, Size)); - if (BufSize < Size) + if (BufSize < Size || !Buffer) { TcpipReleaseSpinLock( &EntityListLock, OldIrql ); /* The buffer is too small to contain requested data, but we return diff --git a/reactos/drivers/network/tcpip/tcpip/ninfo.c b/reactos/drivers/network/tcpip/tcpip/ninfo.c index 958121215fd..1aeffebf671 100644 --- a/reactos/drivers/network/tcpip/tcpip/ninfo.c +++ b/reactos/drivers/network/tcpip/tcpip/ninfo.c @@ -91,20 +91,17 @@ TDI_STATUS InfoTdiQueryGetRouteTable( PNDIS_BUFFER Buffer, PUINT BufferSize ) { RtCount = CopyFIBs( RCache ); while( RtCurrent < RouteEntries + RtCount ) { - /* Copy Desitnation */ + ASSERT(RCacheCur->Router); + RtlCopyMemory( &RtCurrent->Dest, &RCacheCur->NetworkAddress.Address, sizeof(RtCurrent->Dest) ); RtlCopyMemory( &RtCurrent->Mask, &RCacheCur->Netmask.Address, sizeof(RtCurrent->Mask) ); - - if( RCacheCur->Router ) - RtlCopyMemory( &RtCurrent->Gw, - &RCacheCur->Router->Address.Address, - sizeof(RtCurrent->Gw) ); - else - RtlZeroMemory( &RtCurrent->Gw, sizeof(RtCurrent->Gw) ); + RtlCopyMemory( &RtCurrent->Gw, + &RCacheCur->Router->Address.Address, + sizeof(RtCurrent->Gw) ); RtCurrent->Metric1 = RCacheCur->Metric; RtCurrent->Type = TDI_ADDRESS_TYPE_IP;