[ofa-general] [PATCHv2] opensm/mesh/lash: Fix use after free problem in osm_mesh_node_delete

[Sasha Khapyorsky]

Hi Hal,

On 09:51 Fri 31 Jul     , Hal Rosenstock wrote:
> 
> When osm_mesh_node_delete is called, osm_switch_delete may already have
> been called so sw->p_sw is no longer valid to be used although it was
> being used to obtain num_ports.
> 
> Fix this by performing osm_mesh_delete_switches at the end of lash_process.
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
> ---
> Changes since v1:
> Rather than saving num_ports in the mesh node structure on creation and using
> this on deletion, mesh switches deletion should occur at end of the lash
> calculation as none of this state is needed after that 
> Approach proposed by Sasha
> 
> diff --git a/opensm/include/opensm/osm_mesh.h b/opensm/include/opensm/osm_mesh.h
> index 173fa86..89c07e5 100644
> --- a/opensm/include/opensm/osm_mesh.h
> +++ b/opensm/include/opensm/osm_mesh.h
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 2088      System Fabric Works, Inc.
> + * Copyright (c) 2009      HNR Consulting. All rights reserved.
>   *
>   * This software is available to you under a choice of one of two
>   * licenses.  You may choose to be licensed under the terms of the GNU
> @@ -70,6 +71,7 @@ typedef struct _mesh_node {
>  } mesh_node_t;
>  
>  void osm_mesh_node_delete(struct _lash *p_lash, struct _switch *sw);
> +void osm_mesh_delete_switches(struct _lash *p_lash);
>  int osm_mesh_node_create(struct _lash *p_lash, struct _switch *sw);
>  int osm_do_mesh_analysis(struct _lash *p_lash);
>  
> diff --git a/opensm/opensm/osm_mesh.c b/opensm/opensm/osm_mesh.c
> index 23fad87..b22fe6e 100644
> --- a/opensm/opensm/osm_mesh.c
> +++ b/opensm/opensm/osm_mesh.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 2008,2009      System Fabric Works, Inc. All rights reserved.
> + * Copyright (c) 2009           HNR Consulting. All rights reserved.
>   *
>   * This software is available to you under a choice of one of two
>   * licenses.  You may choose to be licensed under the terms of the GNU
> @@ -1358,6 +1359,20 @@ void osm_mesh_node_delete(lash_t *p_lash, switch_t *sw)
>  }
>  
>  /*
> + * osm_mesh_delete_switches - cleanup switches resources
> + */
> +void osm_mesh_delete_switches(lash_t *p_lash)
> +{
> +	if (p_lash->switches) {
> +		unsigned id;
> +		for (id = 0; ((int)id) < p_lash->num_switches; id++)
> +			if (p_lash->switches[id])
> +				osm_mesh_node_delete(p_lash,
> +						     p_lash->switches[id]);
> +	}
> +}

Why should it be in osm_mesh.c? osm_mesh_node_create() and
osm_mesh_node_delete() are called in osm_ucast_lash.c now.

For me it looks that more appropriate place for such cleanup is
lash_free_structures() func in osm_ucast_lash.c.

Sasha