[Openib-windows] Possible race in WSD code

[Tzachi Dar]

Hi Fab,
 
While doing some tests on WSD, I have came to a conclusion that there is
a race in the function ibsp_conn_insert().
 
If I'm right, than the problem is that the code looks at the root of
conn_map without locking first. Since the root might change, until the
lock is taken, I have received situations in which I was inserting at a
time that the p_insert_at was no longer valid.
 
At the end of the function there is a similar situation that is probably
not a bug. Still, to be on the safe side I have also inserted this under
the lock.
 
The attached patch fixes this problem.
 
Thanks
Tzachi
 
Index: core/complib/cl_map.c
===================================================================
--- core/complib/cl_map.c (revision 340)
+++ core/complib/cl_map.c (working copy)
@@ -286,6 +286,7 @@
  CL_ASSERT( p_map->root.p_up == &p_map->root );
  CL_ASSERT( p_map->root.color != CL_MAP_RED );
  CL_ASSERT( p_map->nil.color != CL_MAP_RED );
+ CL_ASSERT(p_insert_at->p_map == p_map);
 
  p_item->p_left = &p_map->nil;
  p_item->p_right = &p_map->nil;
Index: inc/iba/ib_al.h
===================================================================
--- inc/iba/ib_al.h (revision 340)
+++ inc/iba/ib_al.h (working copy)
@@ -8104,6 +8104,34 @@
 
 #define IB_PNP_REG_COMPLETE    IB_PNP_FLAG_REG_COMPLETE
 
+AL_INLINE char * get_pnp_event_name(int event)
+{
+ switch( event )
+ {
+ case IB_PNP_CA_ADD     : return "IB_PNP_CA_ADD";
+ case IB_PNP_CA_REMOVE    : return "IB_PNP_CA_REMOVE";
+ case IB_PNP_PORT_ADD    : return "IB_PNP_PORT_ADD";
+ case IB_PNP_PORT_REMOVE    : return "IB_PNP_PORT_REMOVE";
+ case IB_PNP_PORT_INIT    : return "IB_PNP_PORT_INIT";
+ case IB_PNP_PORT_ARMED    : return "IB_PNP_PORT_ARMED";
+ case IB_PNP_PORT_ACTIVE    : return "IB_PNP_PORT_ACTIVE";
+ case IB_PNP_PORT_DOWN    : return "IB_PNP_PORT_DOWN";
+ case IB_PNP_PKEY_CHANGE    : return "IB_PNP_PKEY_CHANGE";
+ case IB_PNP_SM_CHANGE    : return "IB_PNP_SM_CHANGE";
+ case IB_PNP_GID_CHANGE    : return "IB_PNP_GID_CHANGE";
+ case IB_PNP_LID_CHANGE    : return "IB_PNP_LID_CHANGE";
+ case IB_PNP_SUBNET_TIMEOUT_CHANGE : return
"IB_PNP_SUBNET_TIMEOUT_CHANGE";
+ case IB_PNP_IOU_ADD     : return "IB_PNP_IOU_ADD";
+ case IB_PNP_IOU_REMOVE    : return "IB_PNP_IOU_REMOVE";
+ case IB_PNP_IOC_ADD     : return "IB_PNP_IOC_ADD";
+ case IB_PNP_IOC_REMOVE    : return "IB_PNP_IOC_REMOVE";
+ case IB_PNP_IOC_PATH_ADD   : return "IB_PNP_IOC_PATH_ADD";
+ case IB_PNP_IOC_PATH_REMOVE   : return "IB_PNP_IOC_PATH_REMOVE";
+ case IB_PNP_REG_COMPLETE   : return "IB_PNP_REG_COMPLETE";
+ }
+ return "Unknown";
+}
+
 typedef uint32_t ib_pnp_event_t;
 /*
 * VALUES
Index: ulp/wsd/user/ibsp_iblow.c
===================================================================
--- ulp/wsd/user/ibsp_iblow.c (revision 340)
+++ ulp/wsd/user/ibsp_iblow.c (working copy)
@@ -1206,13 +1206,15 @@
  IN    struct ibsp_socket_info  *s )
 {
  struct ibsp_socket_info  *p_sock;
- cl_rbmap_item_t    *p_item, *p_insert_at;
+ cl_rbmap_item_t   *p_item, *p_insert_at;
  boolean_t     left = TRUE;
+ boolean_t     ret = FALSE;
 
+ cl_spinlock_acquire( &g_ibsp.socket_info_mutex );
  p_item = cl_rbmap_root( &g_ibsp.conn_map );
  p_insert_at = p_item;
 
- cl_spinlock_acquire( &g_ibsp.socket_info_mutex );
+
  CL_ASSERT( !s->conn_item.p_map );
  while( p_item != cl_rbmap_end( &g_ibsp.conn_map ) )
  {
@@ -1247,10 +1249,10 @@
  }
 
  cl_rbmap_insert( &g_ibsp.conn_map, p_insert_at, &s->conn_item, left );
-
+ ret = p_item == cl_rbmap_end( &g_ibsp.conn_map );
 done:
  cl_spinlock_release( &g_ibsp.socket_info_mutex );
- return p_item == cl_rbmap_end( &g_ibsp.conn_map );
+ return ret;
 }
 
 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20060516/9eebb150/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wsd.diff
Type: application/octet-stream
Size: 3105 bytes
Desc: wsd.diff
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20060516/9eebb150/attachment.obj>