On Thu, Jun 18, 2009 at 07:54:22PM +0300, Yossi Etigin wrote:
> [email protected] wrote:
> > @@ -841,10 +841,20 @@ static void ipoib_set_mcast_list(struct net_device 
> > *dev)
> >  static void ipoib_neigh_cleanup(struct neighbour *n)
> >  {
> >     struct ipoib_neigh *neigh;
> > -   struct ipoib_dev_priv *priv = netdev_priv(n->dev);
> > +   struct ipoib_dev_priv *priv;
> >     unsigned long flags;
> >     struct ipoib_ah *ah = NULL;
> >  
> > +   /* 
> > +    * Note that the read of the neigh pointer below is not protected 
> > +    * by a ipoib_dev_priv->lock (since we don't yet know which device's 
> > +    * lock to use). Count on the fact that if ipoib_neigh_free() has 
> > +    * already freed the struct ipoib_neigh, to_ipoib_neigh() will 
> > +    * return NULL.
> > +    * 
> > +    * If to_ipoib_neigh() does not return NULL, we'll re-read neigh 
> > +    * under the appropriate lock below. 
> > +    */
> >     neigh = *to_ipoib_neigh(n);
> >     if (neigh)
> >             priv = netdev_priv(neigh->dev);
> 
> What if someone frees the neighbour right after you assign it to 'neigh'?
> 'neigh->dev' may become invalid, and so is the 'priv' and 'priv->spinlock'.
> 

There remains a window where that can happen, this patch just makes 
that window (quite a bit) smaller.

The best idea I've got for entirely preventing the race was suggested 
here:

http://lists.openfabrics.org/pipermail/general/2009-June/059841.html

(Seems like alot of complexity to add for this, though....)

Suggestions? 

-- 
Arthur

_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to