[ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data

Alex Naslednikov xalex at mellanox.co.il
Thu Aug 26 05:20:40 PDT 2010


Stan,
Please, commit the 2 patches related to OID_GEN_NETWORK_LAYER_ADDRESSES.

The second patch (changes at comment at DHCP code) seems like the part of my last patch for DHCP fix (Linux interop).
I will resend this DHCP patch anyway to be sure it is in because of its importance.

________________________________
From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Thursday, August 26, 2010 3:20 AM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data

Alex,
  Here's the patches from the OpenIB svn tree; testing was OK for ipoib_ndis6_cm[ipoib_driver.cpp.patch+ipoib_port.cpp.patch], ipoib[ndis5-ipoib_driver.cpp.patch] compiles although I did not test.
If these patches look good to you, I will commit to SVN.

stan.


________________________________
From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
Sent: Tuesday, August 24, 2010 11:53 PM
To: Smith, Stan; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data

Hello Stan,
1. It was a typo - the ASSERT should come BEFORE the incrementation of p_net_addr_oid.
There were 2 places, and it was a typo in a one of them.
It also answers you second question - length and type should be checked together before one advanced the p_net_addr_oid pointer.

2. This line :ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
is correct.
In the case when I got the length not-equal to NETWORK_ADDRESS_LENGTH_IP I want to be sure (for debug purposes only) that I have the other type of packets-IPX

3. I am resending the patch again from scratch:


Index: B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c
===================================================================
--- B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision 6295)
+++ B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision 6307)
@@ -2210,30 +2210,29 @@
    cl_vector_get_ptr( &p_adapter->ip_vector, idx );
   p_net_addr_oid = (PNETWORK_ADDRESS)p_net_addrs->Address;

-  for( i = 0; i < p_net_addrs->AddressCount; ++i, p_net_addr_oid =
-   (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-   FIELD_OFFSET(NETWORK_ADDRESS, Address) +
-   p_net_addr_oid->AddressLength) )
+  for( i = 0; i < p_net_addrs->AddressCount; ++i )
   {

-   if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-   {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
-     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-      "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-      NDIS_PROTOCOL_ID_TCP_IP));
-    continue;
-   }
-
+   // Here we check that the data stored at 'AddressLength' field is valid;
+   // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
    if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
    {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
+    IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
      ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
       "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
       NETWORK_ADDRESS_LENGTH_IP));
-    continue;
+    ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+    break;
    }

+   ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
+   p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+        FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+        p_net_addr_oid->AddressLength) ;
+
+
+
    p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;
    if( !cl_memcmp( &p_ip_addr->in_addr,
     &p_addr_item->address.as_ulong, sizeof(ULONG) ) )
@@ -2273,36 +2272,37 @@
  /* Now look for new addresses */
  p_net_addr_oid = (NETWORK_ADDRESS *)p_net_addrs->Address;
  idx = 0;
- for( i = 0; i < p_net_addrs->AddressCount; i++, p_net_addr_oid =
-  (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-  FIELD_OFFSET(NETWORK_ADDRESS, Address) + p_net_addr_oid->AddressLength) )
+
+ for( i = 0; i < p_net_addrs->AddressCount; ++i )
  {

-  if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-  {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
-    ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-     "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-     NDIS_PROTOCOL_ID_TCP_IP));
-   continue;
-  }
-
+  // Here we check that the data stored at 'AddressLength' field is valid;
+  // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
   if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
   {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
+   IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
      "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
      NETWORK_ADDRESS_LENGTH_IP));
-   continue;
+   ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+   break;
+
   }
-
+
+  ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
+  p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+       FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+       p_net_addr_oid->AddressLength) ;
+
+
   p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;

   /* Size the vector as needed. */
   if( cl_vector_get_size( &p_adapter->ip_vector ) <= idx )
    cl_vector_set_size( &p_adapter->ip_vector, idx + 1 );

-  p_addr_item = cl_vector_get_ptr( &p_adapter->ip_vector, idx );
+  p_addr_item = (net_address_item_t *) cl_vector_get_ptr( &p_adapter->ip_vector, idx );
   if( !cl_memcmp( &p_ip_addr->in_addr, &p_addr_item->address.as_ulong,
    sizeof(ULONG) ) )
   {
Index: B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
===================================================================
--- B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp (revision 6295)
+++ B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp (revision 6307)
@@ -4607,7 +4607,6 @@
    if( p_cid[1] == HW_ADDR_LEN+1 && !cl_memcmp( &p_cid[3],
     &s_buf->p_port->p_adapter->params.conf_mac.addr, HW_ADDR_LEN ) )
    {
-    ASSERT( FALSE );
     /* Make sure there's room to extend it.  22 is the size of
      * the CID option for IPoIB. (20 is the length, one byte for type and the second for lenght field)
      */
Index: B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
===================================================================
--- B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision 6295)
+++ B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision 6307)
@@ -3514,30 +3514,28 @@
    cl_vector_get_ptr( &p_adapter->ip_vector, idx );
   p_net_addr_oid = (PNETWORK_ADDRESS)p_net_addrs->Address;

-  for( i = 0; i < p_net_addrs->AddressCount; ++i, p_net_addr_oid =
-   (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-   FIELD_OFFSET(NETWORK_ADDRESS, Address) +
-   p_net_addr_oid->AddressLength) )
+  for( i = 0; i < p_net_addrs->AddressCount; ++i )
   {

-   if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-   {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
-     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-      "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-      NDIS_PROTOCOL_ID_TCP_IP));
-    continue;
-   }
-
+   // Here we check that the data stored at 'AddressLength' field is valid;
+   // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
    if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
    {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
+    IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
      ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
       "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
       NETWORK_ADDRESS_LENGTH_IP));
-    continue;
+    ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+    break;
    }
+
+   ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
+   p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+        FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+        p_net_addr_oid->AddressLength) ;

+
    p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;
    if( !cl_memcmp( &p_ip_addr->in_addr,
     &p_addr_item->address.as_ulong, sizeof(ULONG) ) )
@@ -3577,29 +3575,30 @@
  /* Now look for new addresses */
  p_net_addr_oid = (NETWORK_ADDRESS *)p_net_addrs->Address;
  idx = 0;
- for( i = 0; i < p_net_addrs->AddressCount; i++, p_net_addr_oid =
-  (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-  FIELD_OFFSET(NETWORK_ADDRESS, Address) + p_net_addr_oid->AddressLength) )
+
+ for( i = 0; i < p_net_addrs->AddressCount; ++i )
  {

-  if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-  {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
-    ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-     "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-     NDIS_PROTOCOL_ID_TCP_IP));
-   continue;
-  }
-
+  // Here we check that the data stored at 'AddressLength' field is valid;
+  // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
   if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
   {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
+   IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
      "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
      NETWORK_ADDRESS_LENGTH_IP));
-   continue;
+   ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+   break;
+
   }
-
+
+  ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
+  p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+       FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+       p_net_addr_oid->AddressLength) ;
+
+
   p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;

   /* Size the vector as needed. */

________________________________
From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Tuesday, August 24, 2010 2:10 AM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data

Hello,
  Patch applied and when one attempts to set an IPoIB IF IPv4 address the ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP ); fires.
Immediately prior to the ASSERT() the code

+   p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+        FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+        p_net_addr_oid->AddressLength) ;
This code makes no sense to me in that the original code did not advance the p_net_addr_oid pointer? Why now?
If fact, if the above code is removed, the code performs the desired result in that an IPv4 address can be set on an IPoIB IF without the ASSERT() firing?

Thoughts?

Additionally you likely did not want to use NETWORK_ADDRESS_LENGTH_IPX but wanted NETWORK_ADDRESS_LENGTH_IP  in the following

ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
stan.


________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Monday, August 23, 2010 6:39 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data

Fixing the bug when NDIS sends OID_GEN_NETWORK_LAYER_ADDRESSES with the
list of new addresses with invalid formatting (happened when AddressCount =5)

NDIS sends NETWORK_ADDRESS_LIST structure, which contains an array of NETWORK_ADDRESS structures of variable size.
The calculation of the next address offset is based on AddressLength;
in a case when this field contains wrong data, one can get access violation error

Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)

Index: B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c
===================================================================
--- B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision 6298)
+++ B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision 6299)
@@ -2210,30 +2210,27 @@
    cl_vector_get_ptr( &p_adapter->ip_vector, idx );
   p_net_addr_oid = (PNETWORK_ADDRESS)p_net_addrs->Address;

-  for( i = 0; i < p_net_addrs->AddressCount; ++i, p_net_addr_oid =
-   (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-   FIELD_OFFSET(NETWORK_ADDRESS, Address) +
-   p_net_addr_oid->AddressLength) )
+  for( i = 0; i < p_net_addrs->AddressCount; ++i )
   {

-   if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-   {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
-     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-      "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-      NDIS_PROTOCOL_ID_TCP_IP));
-    continue;
-   }
-
+   // Here we check that the data stored at 'AddressLength' field is valid;
+   // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
    if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
    {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
+    IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
      ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
       "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
       NETWORK_ADDRESS_LENGTH_IP));
-    continue;
+    ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+    break;
    }
+
+   p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+        FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+        p_net_addr_oid->AddressLength) ;

+   ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
    p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;
    if( !cl_memcmp( &p_ip_addr->in_addr,
     &p_addr_item->address.as_ulong, sizeof(ULONG) ) )
@@ -2273,36 +2270,37 @@
  /* Now look for new addresses */
  p_net_addr_oid = (NETWORK_ADDRESS *)p_net_addrs->Address;
  idx = 0;
- for( i = 0; i < p_net_addrs->AddressCount; i++, p_net_addr_oid =
-  (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-  FIELD_OFFSET(NETWORK_ADDRESS, Address) + p_net_addr_oid->AddressLength) )
+
+ for( i = 0; i < p_net_addrs->AddressCount; ++i )
  {

-  if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-  {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
-    ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-     "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-     NDIS_PROTOCOL_ID_TCP_IP));
-   continue;
-  }
-
+  // Here we check that the data stored at 'AddressLength' field is valid;
+  // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
   if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
   {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
+   IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
      "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
      NETWORK_ADDRESS_LENGTH_IP));
-   continue;
+   ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+   break;
+
   }
-
+
+  ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
+  p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+       FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+       p_net_addr_oid->AddressLength) ;
+
+
   p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;

   /* Size the vector as needed. */
   if( cl_vector_get_size( &p_adapter->ip_vector ) <= idx )
    cl_vector_set_size( &p_adapter->ip_vector, idx + 1 );

-  p_addr_item = cl_vector_get_ptr( &p_adapter->ip_vector, idx );
+  p_addr_item = (net_address_item_t *) cl_vector_get_ptr( &p_adapter->ip_vector, idx );
   if( !cl_memcmp( &p_ip_addr->in_addr, &p_addr_item->address.as_ulong,
    sizeof(ULONG) ) )
   {
Index: B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
===================================================================
--- B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision 6298)
+++ B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision 6299)
@@ -3514,30 +3514,27 @@
    cl_vector_get_ptr( &p_adapter->ip_vector, idx );
   p_net_addr_oid = (PNETWORK_ADDRESS)p_net_addrs->Address;

-  for( i = 0; i < p_net_addrs->AddressCount; ++i, p_net_addr_oid =
-   (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-   FIELD_OFFSET(NETWORK_ADDRESS, Address) +
-   p_net_addr_oid->AddressLength) )
+  for( i = 0; i < p_net_addrs->AddressCount; ++i )
   {

-   if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-   {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
-     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-      "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-      NDIS_PROTOCOL_ID_TCP_IP));
-    continue;
-   }
-
+   // Here we check that the data stored at 'AddressLength' field is valid;
+   // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
    if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
    {
-    IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_OID,
+    IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
      ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
       "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
       NETWORK_ADDRESS_LENGTH_IP));
-    continue;
+    ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+    break;
    }
+
+   p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+        FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+        p_net_addr_oid->AddressLength) ;

+   ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
    p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;
    if( !cl_memcmp( &p_ip_addr->in_addr,
     &p_addr_item->address.as_ulong, sizeof(ULONG) ) )
@@ -3577,29 +3574,30 @@
  /* Now look for new addresses */
  p_net_addr_oid = (NETWORK_ADDRESS *)p_net_addrs->Address;
  idx = 0;
- for( i = 0; i < p_net_addrs->AddressCount; i++, p_net_addr_oid =
-  (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
-  FIELD_OFFSET(NETWORK_ADDRESS, Address) + p_net_addr_oid->AddressLength) )
+
+ for( i = 0; i < p_net_addrs->AddressCount; ++i )
  {

-  if( p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP )
-  {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
-    ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X, "
-     "should be 0x%.4X\n", port_num, i, p_net_addr_oid->AddressType,
-     NDIS_PROTOCOL_ID_TCP_IP));
-   continue;
-  }
-
+  // Here we check that the data stored at 'AddressLength' field is valid;
+  // otherwise, it can lead to a memory violation (happened when AddressCount was > 1)
   if( p_net_addr_oid->AddressLength != NETWORK_ADDRESS_LENGTH_IP)
   {
-   IPOIB_PRINT(TRACE_LEVEL_INFORMATION, IPOIB_DBG_OID,
+   IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
     ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d, "
      "should be %d\n", port_num, i, p_net_addr_oid->AddressLength,
      NETWORK_ADDRESS_LENGTH_IP));
-   continue;
+   ASSERT ( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX );
+   break;
+
   }
-
+
+  ASSERT( p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP );
+
+  p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid +
+       FIELD_OFFSET(NETWORK_ADDRESS, Address) +
+       p_net_addr_oid->AddressLength) ;
+
+
   p_ip_addr = (PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;

   /* Size the vector as needed. */
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20100826/081f1d55/attachment.html>


More information about the ofw mailing list