From 03aee138ea10fa0c0d1aa2e02a992590de7b2243 Mon Sep 17 00:00:00 2001 From: ftillier Date: Fri, 29 Jul 2005 05:50:41 +0000 Subject: [PATCH] Fix bug where failed ATS service registration would leave around stale handles which could be freed upon cleanup, causing an access violation (dereferencing freed memory). git-svn-id: svn://openib.tc.cornell.edu/gen1@50 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86 --- trunk/ulp/ipoib/kernel/ipoib_adapter.h | 33 ++++++--- trunk/ulp/ipoib/kernel/ipoib_driver.c | 98 ++++++++++++++++++++------ 2 files changed, 98 insertions(+), 33 deletions(-) diff --git a/trunk/ulp/ipoib/kernel/ipoib_adapter.h b/trunk/ulp/ipoib/kernel/ipoib_adapter.h index c82572cc..fd786434 100644 --- a/trunk/ulp/ipoib/kernel/ipoib_adapter.h +++ b/trunk/ulp/ipoib/kernel/ipoib_adapter.h @@ -265,12 +265,25 @@ typedef struct _ipoib_adapter *********/ -typedef struct _net_address_item +typedef struct _ats_reg { - /* This must be at the start of the structure */ - cl_pool_item_t pool_item; ipoib_adapter_t *p_adapter; ib_reg_svc_handle_t h_reg_svc; + +} ats_reg_t; +/* +* FIELDS +* p_adapter +* Pointer to the adapter to which this address is assigned. +* +* h_reg_svc +* Service registration handle. +*********/ + + +typedef struct _net_address_item +{ + ats_reg_t *p_reg; union _net_address_item_address { ULONG as_ulong; @@ -280,22 +293,22 @@ typedef struct _net_address_item } net_address_item_t; /* * FIELDS -* pool_item -* Used to pool allocation of addresses and link the allocated addresses together -* -* p_adapter -* Pointer to the adapter to which this address is assigned. +* p_reg +* Pointer to the ATS registration assigned to this address. * * address -* Union representing the IP address as an unsigned long or as an array of bytes. +* Union representing the IP address as an unsigned long or as +* an array of bytes. * * as_ulong -* The IP address represented as an unsigned long. Windows stores IPs this way. +* The IP address represented as an unsigned long. Windows stores +* IPs this way. * * as_bytes * The IP address represented as an array of bytes. *********/ + ib_api_status_t ipoib_create_adapter( IN NDIS_HANDLE wrapper_config_context, diff --git a/trunk/ulp/ipoib/kernel/ipoib_driver.c b/trunk/ulp/ipoib/kernel/ipoib_driver.c index 5ad31b27..38ec84e6 100644 --- a/trunk/ulp/ipoib/kernel/ipoib_driver.c +++ b/trunk/ulp/ipoib/kernel/ipoib_driver.c @@ -211,7 +211,7 @@ __ipoib_ats_reg_cb( IN ib_reg_svc_rec_t *p_reg_svc_rec ); static void -__ipoib_ats_unreg_cb( +__ipoib_ats_dereg_cb( IN void *context ); static void @@ -1901,10 +1901,18 @@ __ipoib_set_net_addr( p_addr_item->address.as_bytes[2], p_addr_item->address.as_bytes[3])); - if( p_addr_item->h_reg_svc ) + if( p_addr_item->p_reg ) { - p_adapter->p_ifc->dereg_svc( p_addr_item->h_reg_svc, NULL ); - p_addr_item->h_reg_svc = NULL; + if( p_addr_item->p_reg->h_reg_svc ) + { + p_adapter->p_ifc->dereg_svc( + p_addr_item->p_reg->h_reg_svc, __ipoib_ats_dereg_cb ); + } + else + { + cl_free( p_addr_item->p_reg ); + } + p_addr_item->p_reg = NULL; } p_addr_item->address.as_ulong = 0; } @@ -1955,12 +1963,19 @@ __ipoib_set_net_addr( * Copy the address information, but don't register yet - the port * could be down. */ - p_addr_item->p_adapter = p_adapter; - if( p_addr_item->h_reg_svc ) + if( p_addr_item->p_reg ) { /* If in use by some other address, deregister. */ - p_adapter->p_ifc->dereg_svc( p_addr_item->h_reg_svc, NULL ); - p_addr_item->h_reg_svc = NULL; + if( p_addr_item->p_reg->h_reg_svc ) + { + p_adapter->p_ifc->dereg_svc( + p_addr_item->p_reg->h_reg_svc, __ipoib_ats_dereg_cb ); + } + else + { + cl_free( p_addr_item->p_reg ); + } + p_addr_item->p_reg = NULL; } memcpy ((void *)&p_addr_item->address.as_ulong, (const void *)&p_ip_addr->in_addr, sizeof(ULONG) ); IPOIB_TRACE( IPOIB_DBG_OID | IPOIB_DBG_INFO, @@ -1980,10 +1995,18 @@ __ipoib_set_net_addr( cl_vector_get_ptr( &p_adapter->ip_vector, cl_vector_get_size( &p_adapter->ip_vector ) - 1 ); - if( p_addr_item->h_reg_svc ) + if( p_addr_item->p_reg ) { - p_adapter->p_ifc->dereg_svc( p_addr_item->h_reg_svc, NULL ); - p_addr_item->h_reg_svc = NULL; + if( p_addr_item->p_reg->h_reg_svc ) + { + p_adapter->p_ifc->dereg_svc( + p_addr_item->p_reg->h_reg_svc, __ipoib_ats_dereg_cb ); + } + else + { + cl_free( p_addr_item->p_reg ); + } + p_addr_item->p_reg = NULL; p_addr_item->address.as_ulong = 0; } @@ -2039,7 +2062,6 @@ ipoib_reg_addrs( ib_service.port_guid = p_adapter->guids.port_guid; ib_service.timeout_ms = DEFAULT_SA_TIMEOUT; ib_service.retry_cnt = DEFAULT_SA_RETRIES; - ib_service.svc_context = p_adapter; /* Can't set IB_FLAGS_SYNC here because I can't wait at dispatch */ ib_service.flags = 0; @@ -2062,9 +2084,17 @@ ipoib_reg_addrs( p_addr_item = (net_address_item_t*) cl_vector_get_ptr( &p_adapter->ip_vector, idx ); - if( p_addr_item->h_reg_svc ) + if( p_addr_item->p_reg ) continue; + p_addr_item->p_reg = cl_zalloc( sizeof(ats_reg_t) ); + if( !p_addr_item->p_reg ) + break; + + p_addr_item->p_reg->p_adapter = p_adapter; + + ib_service.svc_context = p_addr_item->p_reg; + ib_service.svc_rec.service_id = ATS_SERVICE_ID & CL_HTON64(0xFFFFFFFFFFFFFF00); /* ATS service IDs start at 0x10000CE100415453 */ @@ -2074,7 +2104,7 @@ ipoib_reg_addrs( p_addr_item->address.as_bytes, IPV4_ADDR_SIZE ); ib_status = p_adapter->p_ifc->reg_svc( - p_adapter->h_al, &ib_service, &p_addr_item->h_reg_svc ); + p_adapter->h_al, &ib_service, &p_addr_item->p_reg->h_reg_svc ); if( ib_status != IB_SUCCESS ) { if( ib_status == IB_INVALID_GUID ) @@ -2104,6 +2134,8 @@ ipoib_reg_addrs( p_adapter->p_ifc->get_err_str( ib_status )) ); p_adapter->hung = TRUE; } + cl_free( p_addr_item->p_reg ); + p_addr_item->p_reg = NULL; } } @@ -2127,12 +2159,19 @@ __ipoib_dereg_addrs( p_addr_item = (net_address_item_t*) cl_vector_get_ptr( &p_adapter->ip_vector, idx ); - if( !p_addr_item->h_reg_svc ) + if( !p_addr_item->p_reg ) continue; - p_adapter->p_ifc->dereg_svc( - p_addr_item->h_reg_svc, NULL ); - p_addr_item->h_reg_svc = NULL; + if( p_addr_item->p_reg->h_reg_svc ) + { + p_adapter->p_ifc->dereg_svc( + p_addr_item->p_reg->h_reg_svc, __ipoib_ats_dereg_cb ); + } + else + { + cl_free( p_addr_item->p_reg ); + } + p_addr_item->p_reg = NULL; } IPOIB_EXIT( IPOIB_DBG_OID ); @@ -2143,7 +2182,7 @@ static void __ipoib_ats_reg_cb( IN ib_reg_svc_rec_t *p_reg_svc_rec ) { - ipoib_adapter_t * p_adapter; + ats_reg_t *p_reg; uint8_t port_num; IPOIB_ENTER( IPOIB_DBG_OID ); @@ -2151,8 +2190,10 @@ __ipoib_ats_reg_cb( CL_ASSERT( p_reg_svc_rec ); CL_ASSERT( p_reg_svc_rec->svc_context ); - p_adapter = (ipoib_adapter_t* __ptr64)p_reg_svc_rec->svc_context; - port_num = IPOIB_ADAPTER_GET_PORT_NUM( p_adapter ); + p_reg = (ats_reg_t* __ptr64)p_reg_svc_rec->svc_context; + port_num = IPOIB_ADAPTER_GET_PORT_NUM( p_reg->p_adapter ); + + cl_obj_lock( &p_reg->p_adapter->obj ); if( p_reg_svc_rec->req_status == IB_SUCCESS && !p_reg_svc_rec->resp_status ) @@ -2176,9 +2217,20 @@ __ipoib_ats_reg_cb( p_reg_svc_rec->svc_rec.service_data8[ATS_IPV4_OFFSET+1], p_reg_svc_rec->svc_rec.service_data8[ATS_IPV4_OFFSET+2], p_reg_svc_rec->svc_rec.service_data8[ATS_IPV4_OFFSET+3], - p_adapter->p_ifc->get_err_str( p_reg_svc_rec->resp_status )) ); - p_adapter->hung = TRUE; + p_reg->p_adapter->p_ifc->get_err_str( p_reg_svc_rec->resp_status )) ); + p_reg->p_adapter->hung = TRUE; + p_reg->h_reg_svc = NULL; } + cl_obj_unlock( &p_reg->p_adapter->obj ); + IPOIB_EXIT( IPOIB_DBG_OID ); } + + +static void +__ipoib_ats_dereg_cb( + IN void *context ) +{ + cl_free( context ); +} -- 2.41.0