[ofw][PATCH] [IBBUS][WinVerbs][SDP] Validation of non-zero size when calling to ExAllocatePoolWithTag

Fab Tillier ftillier at microsoft.com
Mon Oct 26 11:31:23 PDT 2009


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