please consider for 2.6.18 -- thanks! [ATM]: fix possible recursive locking in skb_migrate()
ok this is a real potential deadlock in a way, it takes two locks of 2 skbuffs without doing any kind of lock ordering; I think the following patch should fix it. Just sort the lock taking order by address of the skb.. it's not pretty but it's the best this can do in a minimally invasive way. Signed-off-by: Arjan van de Ven <[EMAIL PROTECTED]> Signed-off-by: Chas Williams <[EMAIL PROTECTED]> --- commit c895a2f42057801ca096f5155e85bc2b2f381250 tree ee64100fdd0302fe5c12ce6c153eea8b94010f2b parent 8bbf3465e23c41b92931e2d2172c184ccd1d2510 author chas williams <[EMAIL PROTECTED]> Fri, 07 Jul 2006 18:11:30 -0400 committer chas williams <[EMAIL PROTECTED]> Fri, 07 Jul 2006 18:11:30 -0400 net/atm/ipcommon.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/net/atm/ipcommon.c b/net/atm/ipcommon.c index 4b1faca..1d3de42 100644 --- a/net/atm/ipcommon.c +++ b/net/atm/ipcommon.c @@ -25,22 +25,27 @@ #endif /* * skb_migrate appends the list at "from" to "to", emptying "from" in the * process. skb_migrate is atomic with respect to all other skb operations on - * "from" and "to". Note that it locks both lists at the same time, so beware - * of potential deadlocks. + * "from" and "to". Note that it locks both lists at the same time, so to deal + * with the lock ordering, the locks are taken in address order. * * This function should live in skbuff.c or skbuff.h. */ -void skb_migrate(struct sk_buff_head *from,struct sk_buff_head *to) +void skb_migrate(struct sk_buff_head *from, struct sk_buff_head *to) { unsigned long flags; struct sk_buff *skb_from = (struct sk_buff *) from; struct sk_buff *skb_to = (struct sk_buff *) to; struct sk_buff *prev; - spin_lock_irqsave(&from->lock,flags); - spin_lock(&to->lock); + if ((unsigned long) from < (unsigned long) to) { + spin_lock_irqsave(&from->lock, flags); + spin_lock_nested(&to->lock, SINGLE_DEPTH_NESTING); + } else { + spin_lock_irqsave(&to->lock, flags); + spin_lock_nested(&from->lock, SINGLE_DEPTH_NESTING); + } prev = from->prev; from->next->prev = to->prev; prev->next = skb_to; @@ -51,7 +56,7 @@ void skb_migrate(struct sk_buff_head *fr from->prev = skb_from; from->next = skb_from; from->qlen = 0; - spin_unlock_irqrestore(&from->lock,flags); + spin_unlock_irqrestore(&from->lock, flags); } - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html