[ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data
Smith, Stan
stan.smith at intel.com
Mon Aug 23 14:40:04 PDT 2010
Hello,
I think Sean is correct in that this sounds like an NDIS problem. That said, NDIS is not going to be fixed soon, so I would agree with your patch for now.
Currently incorporating your patch, will let you know if I can trigger the bug? How is it you trigger the bug which the patch fixes?
thanks,
stan.
PS: anybody know how to convince outlook not to add <tab> chars in the subject line after x characters of subj line?
________________________________
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/20100823/5983dda7/attachment.html>
More information about the ofw
mailing list