[ofa-general] [RFC] ipoib: avoid using stale ipoib_neigh* in ipoib_neigh_cleanup()

[Yossi Etigin]

akepner at sgi.com wrote:
> On Fri, May 22, 2009 at 08:34:57PM +0300, Yossi Etigin wrote:
>> ...
>> Interesting... what does it deadlock with?
> 
> (My previous mail was addressing only the question above. I 
> overlooked what follows.)
> 
>> And what is the hole your fix leaves? 
> 
> Well, in this small window:
> 
> static void ipoib_neigh_cleanup(struct neighbour *n)
> {
>         struct ipoib_neigh *neigh;
>         struct ipoib_dev_priv *priv = netdev_priv(n->dev);
>         unsigned long flags;
>         struct ipoib_ah *ah = NULL;
> 
>         neigh = *to_ipoib_neigh(n); <------- from here
>         if (neigh)
>                 priv = netdev_priv(neigh->dev);
>         else
>                 return;
>         ipoib_dbg(priv,
>                   "neigh_cleanup for %06x %pI6\n",
>                   IPOIB_QPN(n->ha),
>                   n->ha + 4);   <------------ to here
> 	spin_lock_irqsave(&priv->lock, flags);
> 
> 
> we could be using a no-longer-valid neigh.
> 
>> If the (neigh!=NULL) check passes
>> with the spinlock held, shouldn't it be OK to list_del() it?
> 
> Yeah, that should be OK.
> 

So it's a catch.. You can't take priv out of neigh without being 
protected by the spinlock (or someone will kfree the neigh), but you
need priv to get the spinlock in the first place..