[ofa-general] Re: [PATCHv3] opensm/osm_lash: Fix use after free problem in osm_mesh_node_delete

[Hal Rosenstock]

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

> On 06:53 Sun 02 Aug     , Hal Rosenstock wrote:
> > diff --git a/opensm/opensm/osm_ucast_lash.c
> b/opensm/opensm/osm_ucast_lash.c
> > index 1c55a90..cf8e793 100644
> > --- a/opensm/opensm/osm_ucast_lash.c
> > +++ b/opensm/opensm/osm_ucast_lash.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (c) 2007      Simula Research Laboratory. All rights
> reserved.
> >   * Copyright (c) 2007      Silicon Graphics Inc. All rights reserved.
> >   * 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
> > @@ -659,6 +660,18 @@ static void switch_delete(lash_t *p_lash, switch_t *
> sw)
> >       free(sw);
> >  }
> >
> > +static void delete_switches(lash_t *p_lash)
>
> Would delete_mesh_switches() (or cleanup_mesh*()) be a better name? It
> doesn't delete lash's switches, only mesh nodes.


Sure.


>
>
> > +{
> > +     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]);
> > +     }
> > +}
> > +
> > +
> >  static void free_lash_structures(lash_t * p_lash)
> >  {
> >       unsigned int i, j, k;
> > @@ -1219,7 +1232,7 @@ static int lash_process(void *context)
> >
> >       return_status = discover_network_properties(p_lash);
>
> discover_network_properties() can fail in a middle of allocations and
> full clean is desired anyway. It should be safe to 'goto Exit' below
> since mesh node deletion is protected against not yet initialized input.


It's not; I had tried doing that.

-- Hal


>
>
> Sasha
>
> >       if (return_status != IB_SUCCESS)
> > -             goto Exit;
> > +             goto Exit2;
> >
> >       return_status = init_lash_structures(p_lash);
> >       if (return_status != IB_SUCCESS)
> > @@ -1234,6 +1247,9 @@ static int lash_process(void *context)
> >       populate_fwd_tbls(p_lash);
> >
> >  Exit:
> > +     delete_switches(p_lash);
> > +
> > +Exit2:
> >       if (p_lash->vl_min)
> >               free_lash_structures(p_lash);
> >       OSM_LOG_EXIT(p_log);
> >
> _______________________________________________
> 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/d4d8139d/attachment.html>