[openib-general] Re: [PATCH]Repost: IPoIB skb panic

Roland Dreier rdreier at cisco.com
Fri Jun 2 13:09:07 PDT 2006


 > 1. path_free() should call dev_kfree_skb_any() (any context) instead of
 > dev_kfree_skb_irq() (irq context) since it is called in process
 > context. 

Agree -- although actually in the current code, plain dev_kfree_skb()
would be fine.  In fact, since your patch moves the free inside a
spinlock, dev_kfree_skb_irq() would be correct.

 > 2. path->queue should be protected by priv->lock since there is a  race
 > between unicast_send_arp() and ipoib_flush_paths() to release skb when
 > bringing interface down. It's  safe to use priv->lock, because
 > skb_queue_len(&path->queue) <  
 > IPOIB_MAX_PATH_REC_QUEUE, which is 3.

I'm having a hard time understanding this race.  path_free() should
never be called on paths that are reachable via the list of paths or
the rb-tree of paths, and unicast_send_arp() should never touch a path
that is going to path_free().

Also, it seems if there is a race here then this fix is insufficient,
because path_free() does a kfree() on the whole path structure, which
would lead to use-after-free if unicast_send_arp() might still touch it.

So could you diagram the race you are seeing?  (ie what are the two
different threads doing that causes a problem?)

Thanks,
  Roland



More information about the general mailing list