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

[Hal Rosenstock]

Hi Sasha,

On Sun, Aug 2, 2009 at 6:09 AM, Sasha Khapyorsky <sashak at voltaire.com>wrote:

> 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.


Not quite as it cannot be cleaned up until after discover_network_properties
is called and succeeds.

-- Hal


>
>
> Sasha
>  _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20090802/f61c3e60/attachment.html>