Re: dev_kfree_skb() and skb_device_unlock()

David S. Miller (davem@jenolan.rutgers.edu)
Sun, 21 Sep 1997 17:09:13 -0400


From: alan@lxorguk.ukuu.org.uk (Alan Cox)
Date: Sun, 21 Sep 1997 21:46:02 +0100 (BST)

> In v2.0.31pre9 arp.c (arp_find()), I find code snippets like:
>
> if (skb != NULL && !entry) {
> skb_device_unlock(skb); /* else it is lost forever */
> dev_kfree_skb(skb, FREE_WRITE);
> }

I'd say this code is wrong too. I've no idea when that appeared or where
it appeared. If the buffer is unlocked then it should be calling
kfree_skb(). What is more worrying is that it introduces several races and
potential cases where a freed buffer might be sent or double freed.

I added this some time ago because it fixed an skb leak. ANK noticed
this and tried to get me to refresh my memory on the precise problem I
had found which necessitated the unlock there. I couldn't recall
exactly what was going on, but it was some strange interaction with
resolution being attempted from the device packet queueing layer,
after one send attempt had been made already.

Regardless, this did fix the bug people were reporting wrt. skb's
getting lost forever.

It all eminates from the hokey skb locking we do, there are races all
over in to 2.0.x because of this. Most of the skb locking issues have
been resolved in the 2.1.x stack already.

Later,
David "Sparc" Miller
davem@caip.rutgers.edu