From: Huyen Nguyen <[EMAIL PROTECTED]>
A possible bug:
rt_fill_info() calls ipmr_get_route().
ipmr_get_route() calls ipmr_cache_unresolved()
ipmr_cache_unresolved() gets an error and does kfree_skb(skb)
ipmr_cache_unresolved() returns a -ve errno to ipmr_get_route()
ipmr_get_route() returns the errno to rt_fill_info()
rt_fill_info() diddles with the now-kfreed skb.
It may not be a bug if that kfree_skb() is only supposed to be balancing some
skb_get() which we didn't notice. I don't think so, but could someone please
check?
Here's Huyen's proposed fix. I think it goes halfway - it's clearer if the
function which takes a ref on a data structure is responsible for dropping
that ref too, rather than some two-deep callee. IMO.
If we are going to deliberately do funky stuff like this then the skb lifetime
protocol should be commented, so as to avoid confusing poor little non-net
people. IMO.
Signed-off-by: Huyen Nguyen <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
net/ipv4/ipmr.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff -puN net/ipv4/ipmr.c~ipmr_cache_unresolved-fix net/ipv4/ipmr.c
--- devel/net/ipv4/ipmr.c~ipmr_cache_unresolved-fix 2006-03-25
00:32:27.000000000 -0800
+++ devel-akpm/net/ipv4/ipmr.c 2006-03-25 00:32:27.000000000 -0800
@@ -641,8 +641,6 @@ ipmr_cache_unresolved(vifi_t vifi, struc
if (atomic_read(&cache_resolve_queue_len)>=10 ||
(c=ipmr_cache_alloc_unres())==NULL) {
spin_unlock_bh(&mfc_unres_lock);
-
- kfree_skb(skb);
return -ENOBUFS;
}
@@ -663,7 +661,6 @@ ipmr_cache_unresolved(vifi_t vifi, struc
spin_unlock_bh(&mfc_unres_lock);
kmem_cache_free(mrt_cachep, c);
- kfree_skb(skb);
return err;
}
@@ -678,7 +675,6 @@ ipmr_cache_unresolved(vifi_t vifi, struc
* See if we can append the packet
*/
if (c->mfc_un.unres.unresolved.qlen>3) {
- kfree_skb(skb);
err = -ENOBUFS;
} else {
skb_queue_tail(&c->mfc_un.unres.unresolved,skb);
@@ -1391,7 +1387,8 @@ int ip_mr_input(struct sk_buff *skb)
if (vif >= 0) {
int err = ipmr_cache_unresolved(vif, skb);
read_unlock(&mrt_lock);
-
+ if (err < 0)
+ kfree_skb(skb);
return err;
}
read_unlock(&mrt_lock);
_
-
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
