[ofw][PATCH] [IBBUS][WinVerbs][SDP] Validation of non-zero sizewhen calling to ExAllocatePoolWithTag
Alex Naslednikov
xalex at mellanox.co.il
Mon Oct 26 11:35:57 PDT 2009
Because you will get BSOD when running verifier with low-resources.
And why to call this function with zero when it's not advisable ?
-----Original Message-----
From: Fab Tillier [mailto:ftillier at microsoft.com]
Sent: Monday, October 26, 2009 8:31 PM
To: Alex Naslednikov; ofw at lists.openfabrics.org; Leonid Keller
Subject: RE: [ofw][PATCH] [IBBUS][WinVerbs][SDP] Validation of non-zero
sizewhen calling to ExAllocatePoolWithTag
Why not just assert that size isn't zero, rather than suppressing the
error and returning?
-Fab
> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org [mailto:ofw-
> bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
> Sent: Monday, October 26, 2009 11:03 AM
> To: ofw at lists.openfabrics.org; Leonid Keller
> Subject: [ofw][PATCH] [IBBUS][WinVerbs][SDP] Validation of non-zero
> size when calling to ExAllocatePoolWithTag
>
> According to WinDDK, "calling ExAllocatePoolWithTag with memory size
> == 0 will result in pool header wastage"
> In addition, verifier with low mem simulation will crash when calling
> the mentioned function with memory size == 0 This patch fixes this
> problem by replacing unsafe call with appropriate macro signed-off by:
> Alexander Naslednikov (xalex at mellanox.co.il)
>
> Index: D:/windows/MLNX_WinOF_trunk/ulp/sdp/kernel/SdpGenUtils.cpp
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/ulp/sdp/kernel/SdpGenUtils.cpp
> (revision 4987)
> +++ D:/windows/MLNX_WinOF_trunk/ulp/sdp/kernel/SdpGenUtils.cpp
> (revision 4992)
> @@ -372,14 +372,22 @@
> return WSAEINVAL;
> }
> }
> +class ZeroSizePool {
> +} szPool;
>
> void* __cdecl operator new(size_t n ) throw() {
> +
> + //From WinDDK: "Avoid calling with memory size == 0. Doing so will
> result in pool header wastage"
> + // Verifier with low mem simulation will crash with memory size ==
> + 0 if (n ==0) return &szPool;
> ASSERT(n != 0x30);
> return ExAllocatePoolWithTag(NonPagedPool , n,
> GLOBAL_ALLOCATION_TAG); }
>
> void __cdecl operator delete(void* p) {
> - ExFreePoolWithTag(p, GLOBAL_ALLOCATION_TAG);
> + if (p != &szPool)
> + ExFreePoolWithTag(p, GLOBAL_ALLOCATION_TAG);
> }
>
> void* __cdecl operator new(size_t n, void *addr ) throw() {
> Index: D:/windows/MLNX_WinOF_trunk/core/winmad/kernel/wm_driver.c
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/core/winmad/kernel/wm_driver.c
> (revision 4987)
> +++ D:/windows/MLNX_WinOF_trunk/core/winmad/kernel/wm_driver.c
> (revision 4992)
> @@ -238,8 +238,8 @@
> attr = NULL;
> goto out;
> }
> -
> - attr = ExAllocatePoolWithTag(PagedPool, size, 'acmw');
> +
> + attr = ExAllocatePoolWithTagSafeEx(PagedPool, size, 'acmw');
> if (attr == NULL) {
> goto out;
> }
> @@ -269,7 +269,8 @@
> }
>
> size = sizeof(WM_IB_PORT) * attr->num_ports;
> - pDevice->pPortArray = ExAllocatePoolWithTag(PagedPool, size,
> 'pimw');
> +
> + pDevice->pPortArray = ExAllocatePoolWithTagSafeEx(PagedPool, size,
> 'pimw') ;
> if (pDevice->pPortArray == NULL) {
> status = STATUS_NO_MEMORY;
> goto out;
> Index: D:/windows/MLNX_WinOF_trunk/core/winverbs/kernel/wv_device.c
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/core/winverbs/kernel/wv_device.c
> (revision 4987)
> +++ D:/windows/MLNX_WinOF_trunk/core/winverbs/kernel/wv_device.c
> (revision 4992)
> @@ -178,8 +178,8 @@
> attr = NULL;
> goto out;
> }
> -
> - attr = ExAllocatePoolWithTag(PagedPool, size, 'acvw');
> +
> + attr = ExAllocatePoolWithTagSafeEx(PagedPool, size, 'acvw');
> if (attr == NULL) {
> goto out;
> }
> @@ -210,7 +210,7 @@
> pDevice->PortCount = attr->num_ports;
> ExFreePoolWithTag(attr, 'acvw');
>
> - pDevice->pPorts = ExAllocatePoolWithTag(NonPagedPool,
> sizeof(WV_PORT)
> *
> + pDevice->pPorts = ExAllocatePoolWithTagSafeEx(NonPagedPool,
> sizeof(WV_PORT) *
> pDevice->PortCount, 'cpvw');
> if (pDevice->pPorts == NULL) {
> return STATUS_NO_MEMORY;
> Index: D:/windows/MLNX_WinOF_trunk/core/complib/kernel/cl_memory_osd.c
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/core/complib/kernel/cl_memory_osd.c
> (revision 4987)
> +++ D:/windows/MLNX_WinOF_trunk/core/complib/kernel/cl_memory_osd.c
> (revision 4992)
> @@ -38,6 +38,7 @@
> IN const size_t size,
> IN const boolean_t pageable )
> {
> +
> if( pageable )
> {
> CL_ASSERT( KeGetCurrentIrql() < DISPATCH_LEVEL ); @@ -46,7 +47,7 @@
> else
> {
> CL_ASSERT( KeGetCurrentIrql() <= DISPATCH_LEVEL );
> - return( ExAllocatePoolWithTag( NonPagedPool, size, 'virp' ) );
> + return( ExAllocatePoolWithTagSafeEx( NonPagedPool, size, 'virp' )
> + );
> }
> }
>
> Index: D:/windows/MLNX_WinOF_trunk/core/bus/kernel/bus_port_mgr.c
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/core/bus/kernel/bus_port_mgr.c
> (revision 4987)
> +++ D:/windows/MLNX_WinOF_trunk/core/bus/kernel/bus_port_mgr.c
> (revision 4992)
> @@ -1599,7 +1599,7 @@
> dev_id_size = p_ext->pdo.p_pdo_device_info->device_id_size;
>
> /* Device ID is "IBA\SID_<sid> where <sid> is the IO device Service
> ID. */
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size, 'vedq'
> );
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, dev_id_size,
> 'vedq' );
> if( !p_string )
> {
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
> @@ -1635,7 +1635,7 @@
>
> dev_id_size = p_ext->pdo.p_pdo_device_info->hardware_id_size;
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size, 'ihqp'
> );
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, dev_id_size,
> 'ihqp' );
> if( !p_string )
> {
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
> @@ -1669,8 +1669,8 @@
> p_ext = (bus_port_ext_t*)p_dev_obj->DeviceExtension;
>
> dev_id_size = p_ext->pdo.p_pdo_device_info->compatible_id_size;
> -
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size, 'ihqp'
> );
> +
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, dev_id_size,
> 'ihqp' );
> if( !p_string )
> {
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
> @@ -1753,9 +1753,8 @@
> return STATUS_NO_SUCH_DEVICE;
> }
>
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, p_ext-
> >pdo.p_pdo_device_info->description_size, 'edqp' );
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, p_ext-
> >pdo.p_pdo_device_info->description_size, 'edqp' );
> -
> if( !p_string )
> {
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
> Index: D:/windows/MLNX_WinOF_trunk/core/iou/kernel/iou_ioc_mgr.c
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/core/iou/kernel/iou_ioc_mgr.c
> (revision
> 4987)
> +++ D:/windows/MLNX_WinOF_trunk/core/iou/kernel/iou_ioc_mgr.c
> +++ (revision
> 4992)
> @@ -952,8 +952,9 @@
> {
>
> dev_id_size = (p_ext->pdo.p_pdo_device_info)->device_id_size;
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size, 'didq'
> );
>
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, dev_id_size,
> 'didq' );
> +
> if( !p_string )
> {
> IOU_PRINT_EXIT( TRACE_LEVEL_ERROR, IOU_DBG_ERROR, @@ -1027,7
> +1028,7 @@
> {
> hw_id_size = p_ext->pdo.p_pdo_device_info->hardware_id_size;
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, hw_id_size, 'ihqi'
> );
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, hw_id_size,
> 'ihqi' );
> if( !p_string )
> {
> IOU_PRINT_EXIT( TRACE_LEVEL_ERROR, IOU_DBG_ERROR, @@ -1142,9
> +1143,9 @@
> {
>
> compat_id_size = p_ext->pdo.p_pdo_device_info->compatible_id_size;
> +
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool,
> compat_id_size, 'icqi' );
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, compat_id_size,
> 'icqi' );
> -
> if( !p_string )
> {
> IOU_PRINT_EXIT( TRACE_LEVEL_ERROR, IOU_DBG_ERROR, @@ -1302,7
> +1303,7 @@
>
> if ( p_ext->pdo.p_pdo_device_info )
> {
> - p_string = ExAllocatePoolWithTag( NonPagedPool, p_ext-
> >pdo.p_pdo_device_info->description_size,
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, p_ext-
> >pdo.p_pdo_device_info->description_size,
> 'edqi' );
> if( !p_string )
> {
> Index: D:/windows/MLNX_WinOF_trunk/inc/complib/cl_memory.h
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/inc/complib/cl_memory.h (revision
> 4987)
> +++ D:/windows/MLNX_WinOF_trunk/inc/complib/cl_memory.h (revision
> +++ 4992)
> @@ -919,6 +919,21 @@
> /*
> * Define allocation macro.
> */
> +
> +/* From WinDDK: "Avoid calling ExAllocatePoolWithTag with memory size
> == 0.
> + Doing so will result in pool header wastage"
> + Verifier with low mem simulation will crash with memory size == 0
> +*/ #define ExAllocatePoolWithTagSafeEx( pageable, size, tag ) \
> + (size == 0 ? NULL : ExAllocatePoolWithTag(pageable, size, tag))
> +
> +#define ExAllocatePoolWithTagSafeExNonPaged(size, tag ) \
> + (size == 0 ? NULL : ExAllocatePoolWithTag(NonPagedPool, size, tag
> ))
> +
> +#define ExAllocatePoolWithTagSafeExPaged(size, tag ) \
> + (size == 0 ? NULL : ExAllocatePoolWithTag(PagedPool, size, tag ))
> +
> +
> #if defined( CL_TRACK_MEM )
>
> #define cl_malloc( a ) \
> Index: D:/windows/MLNX_WinOF_trunk/etc/kernel/index_list.c
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/etc/kernel/index_list.c (revision
> 4987)
> +++ D:/windows/MLNX_WinOF_trunk/etc/kernel/index_list.c (revision
> +++ 4992)
> @@ -28,7 +28,9 @@
> */
>
> #include "index_list.h"
> +#include <complib/cl_memory.h>
>
> +
> INDEX_ENTRY EmptyList;
>
> static BOOLEAN IndexListGrow(INDEX_LIST *pIndexList) @@ -37,7 +39,8
> @@
> SIZE_T size, i;
>
> size = pIndexList->Size + (PAGE_SIZE / sizeof(INDEX_ENTRY));
> - array = ExAllocatePoolWithTag(NonPagedPool, size *
> sizeof(INDEX_ENTRY), 'xdni');
> +
> + array = ExAllocatePoolWithTagSafeEx(NonPagedPool, size *
> sizeof(INDEX_ENTRY), 'xdni');
> if (array == NULL) {
> return FALSE;
> }
> Index: D:/windows/MLNX_WinOF_trunk/hw/mlx4/kernel/inc/l2w_memory.h
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/hw/mlx4/kernel/inc/l2w_memory.h
> (revision 4987)
> +++ D:/windows/MLNX_WinOF_trunk/hw/mlx4/kernel/inc/l2w_memory.h
> (revision 4992)
> @@ -86,13 +86,13 @@
> ASSERT(bsize);
> switch (gfp_mask) {
> case GFP_ATOMIC:
> - ptr = ExAllocatePoolWithTag( NonPagedPool, bsize, MT_TAG_ATOMIC );
> + ptr = ExAllocatePoolWithTagSafeEx( NonPagedPool, bsize,
> MT_TAG_ATOMIC );
> break;
> case GFP_KERNEL:
> - ptr = ExAllocatePoolWithTag( NonPagedPool, bsize, MT_TAG_KERNEL );
> + ptr = ExAllocatePoolWithTagSafeEx( NonPagedPool, bsize,
> MT_TAG_KERNEL );
> break;
> case GFP_HIGHUSER:
> - ptr = ExAllocatePoolWithTag( NonPagedPool, bsize, MT_TAG_HIGH );
> + ptr = ExAllocatePoolWithTagSafeEx( NonPagedPool, bsize,
> + MT_TAG_HIGH
> );
> break;
> default:
> cl_dbg_out("kmalloc: unsupported flag %d\n", gfp_mask);
> Index: D:/windows/MLNX_WinOF_trunk/hw/mthca/kernel/mt_memory.h
> ===================================================================
> --- D:/windows/MLNX_WinOF_trunk/hw/mthca/kernel/mt_memory.h (revision
> 4987)
> +++ D:/windows/MLNX_WinOF_trunk/hw/mthca/kernel/mt_memory.h (revision
> 4992)
> @@ -52,13 +52,13 @@
> MT_ASSERT( KeGetCurrentIrql() <= DISPATCH_LEVEL );
> switch (gfp_mask) {
> case GFP_ATOMIC:
> - ptr = ExAllocatePoolWithTag( NonPagedPool, bsize, MT_TAG_ATOMIC );
> + ptr = ExAllocatePoolWithTagSafeEx( NonPagedPool, bsize,
> MT_TAG_ATOMIC );
> break;
> case GFP_KERNEL:
> - ptr = ExAllocatePoolWithTag( NonPagedPool, bsize, MT_TAG_KERNEL );
> + ptr = ExAllocatePoolWithTagSafeEx( NonPagedPool, bsize,
> MT_TAG_KERNEL );
> break;
> case GFP_HIGHUSER:
> - ptr = ExAllocatePoolWithTag( NonPagedPool, bsize, MT_TAG_HIGH );
> + ptr = ExAllocatePoolWithTagSafeEx( NonPagedPool, bsize,
> + MT_TAG_HIGH
> );
> break;
> default:
> cl_dbg_out("kmalloc: unsupported flag %d\n", gfp_mask);
More information about the ofw
mailing list