<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META content="text/html; charset=us-ascii" http-equiv=Content-Type>
<META name=GENERATOR content="MSHTML 8.00.6001.18876"></HEAD>
<BODY>
<DIV><SPAN class=343404806-26082010><FONT color=#0000ff size=2
face=Arial>Stan,</FONT></SPAN></DIV>
<DIV><SPAN class=343404806-26082010><FONT size=2 face=Arial><FONT
color=#0000ff>Please, commit the 2 patches related to <FONT
face=Tahoma>OID_GEN_NETWORK_LAYER_ADDRESSES.</FONT></FONT></FONT></SPAN></DIV>
<DIV><SPAN class=343404806-26082010><FONT color=#0000ff size=2
face=Tahoma></FONT></SPAN> </DIV>
<DIV><SPAN class=343404806-26082010><FONT color=#0000ff size=2 face=Tahoma>The
second patch (changes at comment at DHCP code) seems like the part of my last
patch for DHCP fix (Linux interop).</FONT></SPAN></DIV>
<DIV><SPAN class=343404806-26082010><FONT color=#0000ff size=2 face=Tahoma>I
will resend this DHCP patch anyway to be sure it is in because of its
importance.</FONT></SPAN></DIV><BR>
<DIV dir=ltr lang=en-us class=OutlookMessageHeader align=left>
<HR tabIndex=-1>
<FONT size=2 face=Tahoma><B>From:</B> Smith, Stan [mailto:stan.smith@intel.com]
<BR><B>Sent:</B> Thursday, August 26, 2010 3:20 AM<BR><B>To:</B> Alex
Naslednikov; ofw@lists.openfabrics.org<BR><B>Subject:</B> RE: [ofw]
[Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES
contains bad data<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV dir=ltr align=left><FONT color=#0000ff size=2 face=Arial><SPAN
class=762141300-26082010>Alex,</SPAN></FONT></DIV>
<DIV dir=ltr align=left><FONT color=#0000ff size=2 face=Arial><SPAN
class=762141300-26082010> 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.</SPAN></FONT></DIV>
<DIV dir=ltr align=left><FONT color=#0000ff size=2 face=Arial><SPAN
class=762141300-26082010>If these patches look good to you, I will commit to
SVN.</SPAN></FONT></DIV>
<DIV dir=ltr align=left><FONT color=#0000ff size=2 face=Arial><SPAN
class=762141300-26082010></SPAN></FONT> </DIV>
<DIV dir=ltr align=left><FONT color=#0000ff size=2 face=Arial><SPAN
class=762141300-26082010>stan.</SPAN></FONT></DIV>
<DIV dir=ltr align=left><FONT color=#0000ff size=2 face=Arial><SPAN
class=762141300-26082010></SPAN></FONT> </DIV><BR>
<DIV dir=ltr lang=en-us class=OutlookMessageHeader align=left>
<HR tabIndex=-1>
<FONT size=2 face=Tahoma><B>From:</B> Alex Naslednikov
[mailto:xalex@mellanox.co.il] <BR><B>Sent:</B> Tuesday, August 24, 2010 11:53
PM<BR><B>To:</B> Smith, Stan; ofw@lists.openfabrics.org<BR><B>Subject:</B> RE:
[ofw] [Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when
OID_GEN_NETWORK_LAYER_ADDRESSES contains bad data<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial>Hello
Stan,</FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial>1. It
was a typo - the ASSERT should come BEFORE the incrementation of
p_net_addr_oid.</FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial>There
were 2 places, and it was a typo in a one of them.</FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial>It
also answers you second question - length and type should be checked together
before one advanced the p_net_addr_oid pointer.</FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2
face=Arial></FONT></SPAN> </DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial>2.
<FONT color=#000000>This line :<SPAN class=295480323-23082010><FONT size=2
face=Arial>ASSERT ( p_net_addr_oid->AddressLength ==
NETWORK_ADDRESS_LENGTH_IPX );</FONT></SPAN></FONT></FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#000000><SPAN class=295480323-23082010>is
correct.</SPAN></FONT></FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#000000><SPAN class=295480323-23082010>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</SPAN></FONT></FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#000000><SPAN
class=295480323-23082010></SPAN></FONT></FONT></SPAN> </DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#000000><SPAN class=295480323-23082010>3. I am resending the patch again
from scratch:</SPAN></FONT></FONT></SPAN></DIV>
<DIV><SPAN class=779464606-25082010><FONT color=#0000ff size=2
face=Arial><BR></FONT></SPAN><SPAN class=779464606-25082010><FONT color=#0000ff
size=2 face=Arial><BR>Index:
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c<BR>===================================================================<BR>---
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision
6295)<BR>+++
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision
6307)<BR>@@ -2210,30 +2210,29 @@<BR> cl_vector_get_ptr(
&p_adapter->ip_vector, idx );<BR> p_net_addr_oid =
(PNETWORK_ADDRESS)p_net_addrs->Address;<BR> <BR>- for( i = 0;
i < p_net_addrs->AddressCount; ++i, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address)
+<BR>- p_net_addr_oid->AddressLength) )<BR>+ for(
i = 0; i < p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if(
p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING, IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation
(happened when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR> }<BR> <BR>+ ASSERT(
p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP
);<BR>+ <BR>+ p_net_addr_oid =
(PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR>+<BR>+ <BR>+<BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> if(
!cl_memcmp(
&p_ip_addr->in_addr,<BR> &p_addr_item->address.as_ulong,
sizeof(ULONG) ) )<BR>@@ -2273,36 +2272,37 @@<BR> /* Now look for new
addresses */<BR> p_net_addr_oid = (NETWORK_ADDRESS
*)p_net_addrs->Address;<BR> idx = 0;<BR>- for( i = 0; i <
p_net_addrs->AddressCount; i++, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address) +
p_net_addr_oid->AddressLength) )<BR>+ <BR>+ for( i = 0; i <
p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if( p_net_addr_oid->AddressType
!= NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation (happened
when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR>+ <BR> }<BR>-<BR>+ <BR>+ ASSERT(
p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP
);<BR>+ <BR>+ p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t
*)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR>+ <BR>+ <BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> <BR> /*
Size the vector as needed. */<BR> if( cl_vector_get_size(
&p_adapter->ip_vector ) <= idx
)<BR> cl_vector_set_size( &p_adapter->ip_vector,
idx + 1 );<BR> <BR>- p_addr_item = cl_vector_get_ptr(
&p_adapter->ip_vector, idx );<BR>+ p_addr_item =
(net_address_item_t *) cl_vector_get_ptr( &p_adapter->ip_vector, idx
);<BR> if( !cl_memcmp( &p_ip_addr->in_addr,
&p_addr_item->address.as_ulong,<BR> sizeof(ULONG)
) )<BR> {<BR>Index:
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp<BR>===================================================================<BR>---
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp (revision
6295)<BR>+++
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp (revision
6307)<BR>@@ -4607,7 +4607,6 @@<BR> if( p_cid[1] ==
HW_ADDR_LEN+1 && !cl_memcmp(
&p_cid[3],<BR> &s_buf->p_port->p_adapter->params.conf_mac.addr,
HW_ADDR_LEN ) )<BR> {<BR>- ASSERT(
FALSE );<BR> /* Make sure there's room to extend
it. 22 is the size of<BR> * the CID option
for IPoIB. (20 is the length, one byte for type and the second for lenght
field)<BR> */<BR>Index:
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp<BR>===================================================================<BR>---
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision
6295)<BR>+++
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision
6307)<BR>@@ -3514,30 +3514,28 @@<BR> cl_vector_get_ptr(
&p_adapter->ip_vector, idx );<BR> p_net_addr_oid =
(PNETWORK_ADDRESS)p_net_addrs->Address;<BR> <BR>- for( i = 0;
i < p_net_addrs->AddressCount; ++i, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address)
+<BR>- p_net_addr_oid->AddressLength) )<BR>+ for(
i = 0; i < p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if(
p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING, IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation
(happened when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR> }<BR>+ <BR>+ ASSERT(
p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP
);<BR>+ <BR>+ p_net_addr_oid =
(PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR> <BR>+<BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> if(
!cl_memcmp(
&p_ip_addr->in_addr,<BR> &p_addr_item->address.as_ulong,
sizeof(ULONG) ) )<BR>@@ -3577,29 +3575,30 @@<BR> /* Now look for new
addresses */<BR> p_net_addr_oid = (NETWORK_ADDRESS
*)p_net_addrs->Address;<BR> idx = 0;<BR>- for( i = 0; i <
p_net_addrs->AddressCount; i++, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address) +
p_net_addr_oid->AddressLength) )<BR>+ <BR>+ for( i = 0; i <
p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if( p_net_addr_oid->AddressType
!= NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation (happened
when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR>+ <BR> }<BR>-<BR>+ <BR>+ ASSERT(
p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP
);<BR>+ <BR>+ p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t
*)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR>+ <BR>+ <BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> <BR> /*
Size the vector as needed. */<BR></FONT></SPAN></DIV><BR>
<DIV dir=ltr lang=en-us class=OutlookMessageHeader align=left>
<HR tabIndex=-1>
<FONT size=2 face=Tahoma><B>From:</B> Smith, Stan [mailto:stan.smith@intel.com]
<BR><B>Sent:</B> Tuesday, August 24, 2010 2:10 AM<BR><B>To:</B> Alex
Naslednikov; ofw@lists.openfabrics.org<BR><B>Subject:</B> RE: [ofw]
[Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES
contains bad data<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV dir=ltr align=left><SPAN class=295480323-23082010><FONT color=#0000ff
size=2 face=Arial>Hello,</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=295480323-23082010><FONT color=#0000ff
size=2 face=Arial> Patch applied and when one attempts to set an IPoIB IF
IPv4 address the <FONT color=#000000>ASSERT( p_net_addr_oid->AddressType ==
NDIS_PROTOCOL_ID_TCP_IP ); </FONT><FONT
color=#0000ff>fires.</FONT></FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=295480323-23082010><FONT color=#0000ff
size=2 face=Arial><FONT color=#0000ff>Immediately prior to the ASSERT() the
code</FONT></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff></FONT></FONT></SPAN> </DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff><FONT color=#000000>+ p_net_addr_oid =
(PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;</FONT><BR></FONT></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff>This code makes no sense to me in that the original code did not
advance the p_net_addr_oid pointer? Why now?</FONT></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff>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?</FONT></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff></FONT></FONT></SPAN> </DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff>Thoughts?</FONT></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2
face=Arial></FONT></SPAN> </DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2
face=Arial>Additionally you likely did not want to use <FONT
color=#000000>NETWORK_ADDRESS_LENGTH_IPX </FONT><FONT color=#0000ff>but wanted
</FONT><FONT color=#000000>NETWORK_ADDRESS_LENGTH_IP </FONT><FONT
color=#0000ff>in the following</FONT></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010></SPAN><SPAN class=295480323-23082010><FONT
size=2 face=Arial></FONT></SPAN> </DIV>
<DIV><SPAN class=295480323-23082010><FONT size=2 face=Arial>ASSERT (
p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR></FONT></SPAN></DIV>
<DIV><SPAN class=295480323-23082010><FONT color=#0000ff size=2 face=Arial><FONT
color=#0000ff>stan.</DIV>
<DIV dir=ltr align=left><BR></FONT></FONT></SPAN><BR></DIV>
<DIV dir=ltr lang=en-us class=OutlookMessageHeader align=left>
<HR tabIndex=-1>
<FONT size=2 face=Tahoma><B>From:</B> ofw-bounces@lists.openfabrics.org
[mailto:ofw-bounces@lists.openfabrics.org] <B>On Behalf Of </B>Alex
Naslednikov<BR><B>Sent:</B> Monday, August 23, 2010 6:39 AM<BR><B>To:</B>
ofw@lists.openfabrics.org<BR><B>Subject:</B> [ofw]
[Patch][ipoib][ipoib_NDIS6_CM] Fixing a bug when OID_GEN_NETWORK_LAYER_ADDRESSES
contains bad data<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><FONT size=2 face=Arial>Fixing the bug when NDIS sends
OID_GEN_NETWORK_LAYER_ADDRESSES with the <BR>list of new addresses with invalid
formatting (happened when AddressCount =5)<BR> <BR>NDIS sends
NETWORK_ADDRESS_LIST structure, which contains an array of NETWORK_ADDRESS
structures of variable size.<BR>The calculation of the next address offset is
based on AddressLength; <BR>in a case when this field contains wrong data, one
can get access violation error</FONT></DIV>
<DIV> </DIV>
<DIV><FONT size=2 face=Arial>Signed-off by: Alexander Naslednikov (xalex at
mellanox.co.il)</FONT></DIV>
<DIV> </DIV>
<DIV><FONT size=2 face=Arial>Index:
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c<BR>===================================================================<BR>---
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision
6298)<BR>+++
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib/kernel/ipoib_driver.c (revision
6299)<BR>@@ -2210,30 +2210,27 @@<BR> cl_vector_get_ptr(
&p_adapter->ip_vector, idx );<BR> p_net_addr_oid =
(PNETWORK_ADDRESS)p_net_addrs->Address;<BR> <BR>- for( i = 0;
i < p_net_addrs->AddressCount; ++i, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address)
+<BR>- p_net_addr_oid->AddressLength) )<BR>+ for(
i = 0; i < p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if(
p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING, IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation
(happened when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR> }<BR>+ <BR>+ p_net_addr_oid
= (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR> <BR>+ ASSERT( p_net_addr_oid->AddressType ==
NDIS_PROTOCOL_ID_TCP_IP );<BR>+<BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> if(
!cl_memcmp(
&p_ip_addr->in_addr,<BR> &p_addr_item->address.as_ulong,
sizeof(ULONG) ) )<BR>@@ -2273,36 +2270,37 @@<BR> /* Now look for new
addresses */<BR> p_net_addr_oid = (NETWORK_ADDRESS
*)p_net_addrs->Address;<BR> idx = 0;<BR>- for( i = 0; i <
p_net_addrs->AddressCount; i++, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address) +
p_net_addr_oid->AddressLength) )<BR>+ <BR>+ for( i = 0; i <
p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if( p_net_addr_oid->AddressType
!= NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation (happened
when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR>+ <BR> }<BR>-<BR>+ <BR>+ ASSERT(
p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP
);<BR>+ <BR>+ p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t
*)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR>+ <BR>+ <BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> <BR> /*
Size the vector as needed. */<BR> if( cl_vector_get_size(
&p_adapter->ip_vector ) <= idx
)<BR> cl_vector_set_size( &p_adapter->ip_vector,
idx + 1 );<BR> <BR>- p_addr_item = cl_vector_get_ptr(
&p_adapter->ip_vector, idx );<BR>+ p_addr_item =
(net_address_item_t *) cl_vector_get_ptr( &p_adapter->ip_vector, idx
);<BR> if( !cl_memcmp( &p_ip_addr->in_addr,
&p_addr_item->address.as_ulong,<BR> sizeof(ULONG)
) )<BR> {<BR>Index:
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp<BR>===================================================================<BR>---
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision
6298)<BR>+++
B:/users/xalex/MLNX_WinOF-2_1_2/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp (revision
6299)<BR>@@ -3514,30 +3514,27 @@<BR> cl_vector_get_ptr(
&p_adapter->ip_vector, idx );<BR> p_net_addr_oid =
(PNETWORK_ADDRESS)p_net_addrs->Address;<BR> <BR>- for( i = 0;
i < p_net_addrs->AddressCount; ++i, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address)
+<BR>- p_net_addr_oid->AddressLength) )<BR>+ for(
i = 0; i < p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if(
p_net_addr_oid->AddressType != NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING, IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation
(happened when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(
TRACE_LEVEL_WARNING,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR> }<BR>+ <BR>+ p_net_addr_oid
= (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR> <BR>+ ASSERT( p_net_addr_oid->AddressType ==
NDIS_PROTOCOL_ID_TCP_IP );<BR>+<BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> if(
!cl_memcmp(
&p_ip_addr->in_addr,<BR> &p_addr_item->address.as_ulong,
sizeof(ULONG) ) )<BR>@@ -3577,29 +3574,30 @@<BR> /* Now look for new
addresses */<BR> p_net_addr_oid = (NETWORK_ADDRESS
*)p_net_addrs->Address;<BR> idx = 0;<BR>- for( i = 0; i <
p_net_addrs->AddressCount; i++, p_net_addr_oid
=<BR>- (PNETWORK_ADDRESS)((uint8_t *)p_net_addr_oid
+<BR>- FIELD_OFFSET(NETWORK_ADDRESS, Address) +
p_net_addr_oid->AddressLength) )<BR>+ <BR>+ for( i = 0; i <
p_net_addrs->AddressCount; ++i
)<BR> {<BR> <BR>- if( p_net_addr_oid->AddressType
!= NDIS_PROTOCOL_ID_TCP_IP
)<BR>- {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>- ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong type of 0x%.4X,
"<BR>- "should be 0x%.4X\n", port_num, i,
p_net_addr_oid->AddressType,<BR>- NDIS_PROTOCOL_ID_TCP_IP));<BR>- continue;<BR>- }<BR>-<BR>+ //
Here we check that the data stored at 'AddressLength' field is
valid;<BR>+ // otherwise, it can lead to a memory violation (happened
when AddressCount was > 1)<BR> if(
p_net_addr_oid->AddressLength !=
NETWORK_ADDRESS_LENGTH_IP)<BR> {<BR>- IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
IPOIB_DBG_OID,<BR>+ IPOIB_PRINT(TRACE_LEVEL_ERROR,
IPOIB_DBG_ERROR,<BR> ("Port %d
OID_GEN_NETWORK_LAYER_ADDRESSES - Address %d is wrong size of %d,
"<BR> "should be %d\n", port_num, i,
p_net_addr_oid->AddressLength,<BR> NETWORK_ADDRESS_LENGTH_IP));<BR>- continue;<BR>+ ASSERT
( p_net_addr_oid->AddressLength == NETWORK_ADDRESS_LENGTH_IPX
);<BR>+ break;<BR>+ <BR> }<BR>-<BR>+ <BR>+ ASSERT(
p_net_addr_oid->AddressType == NDIS_PROTOCOL_ID_TCP_IP
);<BR>+ <BR>+ p_net_addr_oid = (PNETWORK_ADDRESS)((uint8_t
*)p_net_addr_oid
+<BR>+ FIELD_OFFSET(NETWORK_ADDRESS,
Address)
+<BR>+ p_net_addr_oid->AddressLength)
;<BR>+ <BR>+ <BR> p_ip_addr =
(PNETWORK_ADDRESS_IP)p_net_addr_oid->Address;<BR> <BR> /*
Size the vector as needed. */<BR></FONT></DIV></BODY></HTML>