[openib-general] [PATCH] Opensm - bug in osm_sa_path_record with 0 records

[Yael Kalka]

Hi Hal,

During some testing of path record we found a bug in the code.
If the number of records return is zero, then there is clearing of 
non allocated memory.
I've added some changes to the __osm_pr_rcv_respond function, to match
other sa responses.
Attached is a patch to fix it.
Thanks,
Yael

Thanks,
Yael

Signed-off-by:  Yael Kalka <yael at mellanox.co.il>

Index: opensm/osm_sa_path_record.c
===================================================================
--- opensm/osm_sa_path_record.c	(revision 3955)
+++ opensm/osm_sa_path_record.c	(working copy)
@@ -1448,7 +1448,7 @@ __osm_pr_rcv_respond(
   osm_madw_t*              p_resp_madw;
   const ib_sa_mad_t*       p_sa_mad;
   ib_sa_mad_t*          p_resp_sa_mad;
-  size_t                num_rec, num_copied;
+  size_t                num_rec, num_copied, pre_trim_num_rec;
 #ifndef VENDOR_RMPP_SUPPORT
   size_t		trim_num_rec;
 #endif
@@ -1456,6 +1456,7 @@ __osm_pr_rcv_respond(
   ib_api_status_t          status;
   const ib_sa_mad_t*    p_rcvd_mad = osm_madw_get_sa_mad_ptr( p_madw );
   osm_pr_item_t*        p_pr_item;
+  uint32_t              i;
 
   OSM_LOG_ENTER( p_rcv->p_log, __osm_pr_rcv_respond );
 
@@ -1483,6 +1484,7 @@ __osm_pr_rcv_respond(
     goto Exit;
   }
 
+  pre_trim_num_rec = num_rec;
 #ifndef VENDOR_RMPP_SUPPORT
   trim_num_rec = (MAD_BLOCK_SIZE - IB_SA_MAD_HDR_SIZE) / sizeof(ib_path_rec_t);
   if (trim_num_rec < num_rec)
@@ -1495,11 +1497,15 @@ __osm_pr_rcv_respond(
   }
 #endif
 
-  if( osm_log_is_active( p_rcv->p_log, OSM_LOG_DEBUG ) )
-  {
     osm_log( p_rcv->p_log, OSM_LOG_DEBUG,
              "__osm_pr_rcv_respond: "
              "Generating response with %u records.\n", num_rec );
+
+  if ((p_rcvd_mad->method == IB_MAD_METHOD_GET) && (num_rec == 0))
+  {
+    osm_sa_send_error( p_rcv->p_resp, p_madw,
+                       IB_SA_MAD_STATUS_NO_RECORDS );
+    goto Exit;
   }
 
   /*
@@ -1514,6 +1520,16 @@ __osm_pr_rcv_respond(
     osm_log( p_rcv->p_log, OSM_LOG_ERROR,
              "__osm_pr_rcv_respond: ERR 1F14: "
              "Unable to allocate MAD.\n" );
+
+    for( i = 0; i < num_rec; i++ )
+    {
+      p_pr_item = (osm_pr_item_t*)cl_qlist_remove_head( p_list );
+      cl_qlock_pool_put( &p_rcv->pr_pool, &p_pr_item->pool_item );
+    }
+
+    osm_sa_send_error( p_rcv->p_resp, p_madw,
+                       IB_SA_MAD_STATUS_NO_RESOURCES );
+
     goto Exit;
   }
 
@@ -1528,6 +1544,8 @@ __osm_pr_rcv_respond(
   p_resp_sa_mad->attr_offset =
     ib_get_attr_offset( sizeof(ib_path_rec_t) );
 
+  p_resp_pr = (ib_path_rec_t*)ib_sa_mad_get_payload_ptr( p_resp_sa_mad );
+
 #ifndef VENDOR_RMPP_SUPPORT
   /* we support only one packet RMPP - so we will set the first and
      last flags for gettable */
@@ -1542,37 +1560,19 @@ __osm_pr_rcv_respond(
     p_resp_sa_mad->rmpp_flags = IB_RMPP_FLAG_ACTIVE;
 #endif
 
-  p_resp_pr = (ib_path_rec_t*)ib_sa_mad_get_payload_ptr( p_resp_sa_mad );
-
-  if ( num_rec == 0 )
-  {
-    if (p_resp_sa_mad->method == IB_MAD_METHOD_GET_RESP)
-      p_resp_sa_mad->status = IB_SA_MAD_STATUS_NO_RECORDS;
-    cl_memclr( p_resp_pr, sizeof(*p_resp_pr) );
-  }
-  else
+  for ( i = 0; i < pre_trim_num_rec; i++ )
   {
     p_pr_item = (osm_pr_item_t*)cl_qlist_remove_head( p_list );
-
-    /* we need to track the number of copied items so we can
-     * stop the copy - but clear them all
-     */
-    num_copied = 0;
-
-    while( p_pr_item != (osm_pr_item_t*)cl_qlist_end( p_list ) )
-    {
-      /*  Copy the Path Records from the list into the MAD */
-      if (num_copied < num_rec)
-      {
+    /* copy only if not trimmed */
+    if (i < num_rec)
         *p_resp_pr = p_pr_item->path_rec;
-        num_copied++;
-      }
+
       cl_qlock_pool_put( &p_rcv->pr_pool, &p_pr_item->pool_item );
       p_resp_pr++;
-      p_pr_item = (osm_pr_item_t*)cl_qlist_remove_head( p_list );
-    }
   }
 
+  CL_ASSERT( cl_is_qlist_empty( p_list ) );
+
   status = osm_vendor_send( p_resp_madw->h_bind, p_resp_madw, FALSE );
 
   if( status != IB_SUCCESS )