win-pvdrivers

changeset 612:4d777ca3cdd1

Fix a problem with pointer arithmetic that could cause a crash under high load
author James Harper <james.harper@bendigoit.com.au>
date Wed Jul 15 19:55:14 2009 +1000 (2009-07-15)
parents 8fdba34031a8
children 227085ff4ffd
files xenpci/xenpci_pdo.c
line diff
     1.1 --- a/xenpci/xenpci_pdo.c	Wed Jul 15 19:54:03 2009 +1000
     1.2 +++ b/xenpci/xenpci_pdo.c	Wed Jul 15 19:55:14 2009 +1000
     1.3 @@ -364,10 +364,10 @@ XenPci_DOP_MapTransfer(
     1.4    {
     1.5    case MAP_TYPE_MDL:
     1.6      //KdPrint((__DRIVER_NAME "     MAP_TYPE_MDL\n"));
     1.7 -    mdl_offset = (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)MmGetMdlVirtualAddress(mdl));
     1.8 +    mdl_offset = (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)MmGetMdlVirtualAddress(mdl));
     1.9      page_offset = PtrToUlong(CurrentVa) & (PAGE_SIZE - 1);
    1.10      *Length = min(*Length, PAGE_SIZE - page_offset);
    1.11 -    pfn_index = (ULONG)(((ULONGLONG)CurrentVa >> PAGE_SHIFT) - ((ULONGLONG)MmGetMdlVirtualAddress(mdl) >> PAGE_SHIFT));
    1.12 +    pfn_index = (ULONG)(((UINT_PTR)CurrentVa >> PAGE_SHIFT) - ((UINT_PTR)MmGetMdlVirtualAddress(mdl) >> PAGE_SHIFT));
    1.13      //KdPrint((__DRIVER_NAME "     mdl_offset = %d, page_offset = %d, length = %d, pfn_index = %d\n",
    1.14      //  mdl_offset, page_offset, *Length, pfn_index));
    1.15      pfn = MmGetMdlPfnArray(mdl)[pfn_index];
    1.16 @@ -381,7 +381,7 @@ XenPci_DOP_MapTransfer(
    1.17      //KdPrint((__DRIVER_NAME "     MAP_TYPE_REMAPPED (MapTransfer)\n"));
    1.18      //KdPrint((__DRIVER_NAME "     Mdl = %p, MapRegisterBase = %p, MdlVa = %p, CurrentVa = %p, Length = %d\n",
    1.19      //  mdl, MapRegisterBase, MmGetMdlVirtualAddress(mdl), CurrentVa, *Length));
    1.20 -    mdl_offset = (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)MmGetMdlVirtualAddress(mdl));
    1.21 +    mdl_offset = (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)MmGetMdlVirtualAddress(mdl));
    1.22      *Length = min(*Length, PAGE_SIZE);
    1.23      map_register->aligned_buffer = ExAllocatePoolWithTag(NonPagedPool, PAGE_SIZE, XENPCI_POOL_TAG);
    1.24      ASSERT(map_register->aligned_buffer);
    1.25 @@ -481,11 +481,12 @@ XenPci_DOP_PutScatterGatherList(
    1.26          PVOID mdl_start_va = MmGetMdlVirtualAddress(curr_mdl);
    1.27          ULONG mdl_byte_count = MmGetMdlByteCount(curr_mdl);
    1.28          ULONG mdl_offset = 0;
    1.29 -        if ((ULONGLONG)sg_extra->currentva >= (ULONGLONG)mdl_start_va && (ULONGLONG)sg_extra->currentva < (ULONGLONG)mdl_start_va + mdl_byte_count)
    1.30 +        /* need to use <= va + len - 1 to avoid ptr wraparound */
    1.31 +        if ((UINT_PTR)sg_extra->currentva >= (UINT_PTR)mdl_start_va && (UINT_PTR)sg_extra->currentva <= (UINT_PTR)mdl_start_va + mdl_byte_count - 1)
    1.32          {
    1.33            active = TRUE;
    1.34 -          mdl_byte_count -= (ULONG)((ULONGLONG)sg_extra->currentva - (ULONGLONG)mdl_start_va);
    1.35 -          mdl_offset = (ULONG)((ULONGLONG)sg_extra->currentva - (ULONGLONG)mdl_start_va);
    1.36 +          mdl_byte_count -= (ULONG)((UINT_PTR)sg_extra->currentva - (UINT_PTR)mdl_start_va);
    1.37 +          mdl_offset = (ULONG)((UINT_PTR)sg_extra->currentva - (UINT_PTR)mdl_start_va);
    1.38            mdl_start_va = sg_extra->currentva;
    1.39          }
    1.40          if (active)
    1.41 @@ -546,11 +547,11 @@ XenPci_DOP_CalculateScatterGatherList(
    1.42      //  KdPrint((__DRIVER_NAME "     CurrentVa (%p) != MdlVa (%p)\n", CurrentVa, MmGetMdlVirtualAddress(Mdl)));
    1.43      //
    1.44  
    1.45 -    KdPrint((__DRIVER_NAME "     CurrentVa = %p, MdlVa = %p\n", CurrentVa, MmGetMdlVirtualAddress(Mdl)));
    1.46 +    //KdPrint((__DRIVER_NAME "     CurrentVa = %p, MdlVa = %p\n", CurrentVa, MmGetMdlVirtualAddress(Mdl)));
    1.47  
    1.48      for (curr_mdl = Mdl, elements = 0; curr_mdl; curr_mdl = curr_mdl->Next)
    1.49      {
    1.50 -      KdPrint((__DRIVER_NAME "     curr_mdlVa = %p, curr_mdl size = %d\n", MmGetMdlVirtualAddress(curr_mdl), MmGetMdlByteCount(curr_mdl)));
    1.51 +      //KdPrint((__DRIVER_NAME "     curr_mdlVa = %p, curr_mdl size = %d\n", MmGetMdlVirtualAddress(curr_mdl), MmGetMdlByteCount(curr_mdl)));
    1.52        elements += ADDRESS_AND_SIZE_TO_SPAN_PAGES(MmGetMdlVirtualAddress(curr_mdl), MmGetMdlByteCount(curr_mdl));
    1.53      }
    1.54    }
    1.55 @@ -561,7 +562,7 @@ XenPci_DOP_CalculateScatterGatherList(
    1.56  
    1.57    if (elements > xen_dma_adapter->adapter_object.MapRegistersPerChannel)
    1.58    {
    1.59 -    KdPrint((__DRIVER_NAME "     elements = %d - too many\n", elements));
    1.60 +    //KdPrint((__DRIVER_NAME "     elements = %d - too many\n", elements));
    1.61      if (NumberOfMapRegisters)
    1.62        *NumberOfMapRegisters = 0;
    1.63      *ScatterGatherListSize = 0;
    1.64 @@ -575,7 +576,7 @@ XenPci_DOP_CalculateScatterGatherList(
    1.65    if (NumberOfMapRegisters)
    1.66      *NumberOfMapRegisters = elements;
    1.67  
    1.68 -  KdPrint((__DRIVER_NAME "     ScatterGatherListSize = %d, NumberOfMapRegisters = %d\n", *ScatterGatherListSize, elements));
    1.69 +  //KdPrint((__DRIVER_NAME "     ScatterGatherListSize = %d, NumberOfMapRegisters = %d\n", *ScatterGatherListSize, elements));
    1.70  
    1.71    //FUNCTION_EXIT();
    1.72    return STATUS_SUCCESS;
    1.73 @@ -651,15 +652,16 @@ XenPci_DOP_BuildScatterGatherListButDont
    1.74          {
    1.75            mdl_start_va = MmGetMdlVirtualAddress(curr_mdl);
    1.76            mdl_byte_count = MmGetMdlByteCount(curr_mdl);
    1.77 -          if ((ULONGLONG)CurrentVa >= (ULONGLONG)mdl_start_va && (ULONGLONG)CurrentVa < (ULONGLONG)mdl_start_va + mdl_byte_count)
    1.78 +          /* need to use <= va + len - 1 to avoid ptr wraparound */
    1.79 +          if ((UINT_PTR)CurrentVa >= (UINT_PTR)mdl_start_va && (UINT_PTR)CurrentVa <= (UINT_PTR)mdl_start_va + mdl_byte_count - 1)
    1.80            {
    1.81              active = TRUE;
    1.82 -            mdl_byte_count -= (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)mdl_start_va);
    1.83 +            mdl_byte_count -= (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)mdl_start_va);
    1.84              mdl_start_va = CurrentVa;
    1.85            }
    1.86            if (active)
    1.87            {
    1.88 -            if (((ULONGLONG)mdl_start_va & (alignment - 1)) || (mdl_byte_count & (alignment - 1)))
    1.89 +            if (((UINT_PTR)mdl_start_va & (alignment - 1)) || (mdl_byte_count & (alignment - 1)))
    1.90                map_type = MAP_TYPE_REMAPPED;
    1.91              remapped_bytes += mdl_byte_count;
    1.92            }
    1.93 @@ -677,21 +679,33 @@ XenPci_DOP_BuildScatterGatherListButDont
    1.94      map_type = MAP_TYPE_MDL;
    1.95    }
    1.96    if (map_type == MAP_TYPE_MDL)
    1.97 -  {
    1.98 -    for (curr_mdl = Mdl, sglist->NumberOfElements = 0, active = FALSE; curr_mdl; curr_mdl = curr_mdl->Next)
    1.99 +  {    
   1.100 +    for (curr_mdl = Mdl, sglist->NumberOfElements = 0, total_remaining = Length, active = FALSE; total_remaining > 0; curr_mdl = curr_mdl->Next)
   1.101      {
   1.102 +      if (!curr_mdl)
   1.103 +      {
   1.104 +      KdPrint((__DRIVER_NAME "     CurrentVa = %p, Length = %d\n", CurrentVa, Length));
   1.105 +for (curr_mdl = Mdl; curr_mdl; curr_mdl = curr_mdl->Next)
   1.106 +{
   1.107 +  KdPrint((__DRIVER_NAME "     Mdl = %p, VirtualAddress = %p, ByteCount = %d\n", Mdl, MmGetMdlVirtualAddress(curr_mdl), MmGetMdlByteCount(curr_mdl)));
   1.108 +}
   1.109 +      }
   1.110 +      ASSERT(curr_mdl);
   1.111        mdl_start_va = MmGetMdlVirtualAddress(curr_mdl);
   1.112        mdl_byte_count = MmGetMdlByteCount(curr_mdl);
   1.113 -      if ((ULONGLONG)CurrentVa >= (ULONGLONG)mdl_start_va && (ULONGLONG)CurrentVa < (ULONGLONG)mdl_start_va + mdl_byte_count)
   1.114 +      /* need to use <= va + len - 1 to avoid ptr wraparound */
   1.115 +      if ((UINT_PTR)CurrentVa >= (UINT_PTR)mdl_start_va && (UINT_PTR)CurrentVa <= (UINT_PTR)mdl_start_va + mdl_byte_count - 1)
   1.116        {
   1.117          active = TRUE;
   1.118 -        mdl_byte_count -= (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)mdl_start_va);
   1.119 +        mdl_byte_count -= (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)mdl_start_va);
   1.120          mdl_start_va = CurrentVa;
   1.121        }
   1.122 +      mdl_byte_count = min(mdl_byte_count, total_remaining);
   1.123        if (active)
   1.124        {
   1.125          sglist->NumberOfElements += ADDRESS_AND_SIZE_TO_SPAN_PAGES(
   1.126 -          MmGetMdlVirtualAddress(curr_mdl), MmGetMdlByteCount(curr_mdl));
   1.127 +          mdl_start_va, mdl_byte_count);
   1.128 +        total_remaining -= mdl_byte_count;
   1.129        }
   1.130      }
   1.131    }
   1.132 @@ -714,23 +728,26 @@ XenPci_DOP_BuildScatterGatherListButDont
   1.133    case MAP_TYPE_MDL:
   1.134      //KdPrint((__DRIVER_NAME "     MAP_TYPE_MDL - %p\n", MmGetMdlVirtualAddress(Mdl)));
   1.135      total_remaining = Length;
   1.136 -    for (sg_element = 0, curr_mdl = Mdl, active = FALSE; curr_mdl; curr_mdl = curr_mdl->Next)
   1.137 +    for (sg_element = 0, curr_mdl = Mdl, active = FALSE; total_remaining > 0; curr_mdl = curr_mdl->Next)
   1.138      {
   1.139 +      ASSERT(curr_mdl);
   1.140        mdl_start_va = MmGetMdlVirtualAddress(curr_mdl);
   1.141        mdl_byte_count = MmGetMdlByteCount(curr_mdl);
   1.142 -      if ((ULONGLONG)CurrentVa >= (ULONGLONG)mdl_start_va && (ULONGLONG)CurrentVa < (ULONGLONG)mdl_start_va + mdl_byte_count)
   1.143 +      /* need to use <= va + len - 1 to avoid ptr wraparound */
   1.144 +      if ((UINT_PTR)CurrentVa >= (UINT_PTR)mdl_start_va && (UINT_PTR)CurrentVa <= (UINT_PTR)mdl_start_va + mdl_byte_count - 1)
   1.145        {
   1.146          active = TRUE;
   1.147 -        mdl_byte_count -= (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)mdl_start_va);
   1.148 +        mdl_byte_count -= (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)mdl_start_va);
   1.149          mdl_start_va = CurrentVa;
   1.150        }
   1.151        if (active)
   1.152        {
   1.153          ULONG pfn_offset;
   1.154 -        remaining = mdl_byte_count;
   1.155 -        offset = (ULONG)((ULONGLONG)mdl_start_va & (PAGE_SIZE - 1));
   1.156 -        pfn_offset = (ULONG)(((ULONGLONG)mdl_start_va >> PAGE_SHIFT) - ((ULONGLONG)MmGetMdlVirtualAddress(curr_mdl) >> PAGE_SHIFT));
   1.157 -        for (i = 0; i < ADDRESS_AND_SIZE_TO_SPAN_PAGES(mdl_start_va, mdl_byte_count); i++)
   1.158 +        remaining = min(mdl_byte_count, total_remaining);
   1.159 +        offset = (ULONG)((UINT_PTR)mdl_start_va & (PAGE_SIZE - 1));
   1.160 +        pfn_offset = (ULONG)(((UINT_PTR)mdl_start_va >> PAGE_SHIFT) - ((UINT_PTR)MmGetMdlVirtualAddress(curr_mdl) >> PAGE_SHIFT));
   1.161 +        //for (i = 0; i < ADDRESS_AND_SIZE_TO_SPAN_PAGES(mdl_start_va, mdl_byte_count); i++)
   1.162 +        for (i = 0; remaining > 0; i++)
   1.163          {
   1.164            pfn = MmGetMdlPfnArray(curr_mdl)[pfn_offset + i];
   1.165            ASSERT(pfn);
   1.166 @@ -745,6 +762,16 @@ XenPci_DOP_BuildScatterGatherListButDont
   1.167          }
   1.168        }
   1.169      }
   1.170 +    if (sg_element != sglist->NumberOfElements)
   1.171 +    {
   1.172 +      KdPrint((__DRIVER_NAME "     sg_element = %d, sglist->NumberOfElements = %d\n", sg_element, sglist->NumberOfElements));
   1.173 +      KdPrint((__DRIVER_NAME "     CurrentVa = %p, Length = %d\n", CurrentVa, Length));
   1.174 +for (curr_mdl = Mdl; curr_mdl; curr_mdl = curr_mdl->Next)
   1.175 +{
   1.176 +  KdPrint((__DRIVER_NAME "     Mdl = %p, VirtualAddress = %p, ByteCount = %d\n", Mdl, MmGetMdlVirtualAddress(curr_mdl), MmGetMdlByteCount(curr_mdl)));
   1.177 +}
   1.178 +    }
   1.179 +    ASSERT(sg_element == sglist->NumberOfElements);
   1.180      break;
   1.181    case MAP_TYPE_REMAPPED:
   1.182      sg_extra->aligned_buffer = ExAllocatePoolWithTag(NonPagedPool, max(remapped_bytes, PAGE_SIZE), XENPCI_POOL_TAG);
   1.183 @@ -753,7 +780,7 @@ XenPci_DOP_BuildScatterGatherListButDont
   1.184        KdPrint((__DRIVER_NAME "     MAP_TYPE_REMAPPED buffer allocation failed - requested va = %p, length = %d\n", MmGetMdlVirtualAddress(Mdl), remapped_bytes));
   1.185        return STATUS_INSUFFICIENT_RESOURCES;
   1.186      }
   1.187 -    //KdPrint((__DRIVER_NAME "     MAP_TYPE_REMAPPED - %p -> %p\n", MmGetMdlVirtualAddress(Mdl), sg_extra->aligned_buffer));
   1.188 +    //KdPrint((__DRIVER_NAME "     MAP_TYPE_REMAPPED - %p\n", sg_extra->aligned_buffer));
   1.189      sg_extra->mdl = Mdl;
   1.190      sg_extra->currentva = CurrentVa;
   1.191      sg_extra->copy_length = remapped_bytes;
   1.192 @@ -765,11 +792,12 @@ XenPci_DOP_BuildScatterGatherListButDont
   1.193          mdl_start_va = MmGetMdlVirtualAddress(curr_mdl);
   1.194          mdl_byte_count = MmGetMdlByteCount(curr_mdl);
   1.195          mdl_offset = 0;
   1.196 -        if ((ULONGLONG)CurrentVa >= (ULONGLONG)mdl_start_va && (ULONGLONG)CurrentVa < (ULONGLONG)mdl_start_va + mdl_byte_count)
   1.197 +        /* need to use <= va + len - 1 to avoid ptr wraparound */
   1.198 +        if ((UINT_PTR)CurrentVa >= (UINT_PTR)mdl_start_va && (UINT_PTR)CurrentVa <= (UINT_PTR)mdl_start_va + mdl_byte_count - 1)
   1.199          {
   1.200            active = TRUE;
   1.201 -          mdl_byte_count -= (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)mdl_start_va);
   1.202 -          mdl_offset = (ULONG)((ULONGLONG)CurrentVa - (ULONGLONG)mdl_start_va);
   1.203 +          mdl_byte_count -= (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)mdl_start_va);
   1.204 +          mdl_offset = (ULONG)((UINT_PTR)CurrentVa - (UINT_PTR)mdl_start_va);
   1.205            mdl_start_va = CurrentVa;
   1.206          }
   1.207          if (active)