Message ID | 1456748573-21586-1-git-send-email-zhangshengju@cmss.chinamobile.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
Hello. On 2/29/2016 3:22 PM, Zhang Shengju wrote: > If skb_reorder_vlan_header() failed, skb is freed and NULL is returned. > Then at skb_vlan_untag(), it will free skbuff again which cause double > free. > > This patch removes kfree_skb() call in function skb_reorder_vlan_header(). > > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> > --- > net/core/skbuff.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 488566b..1312d4b 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4350,7 +4350,6 @@ EXPORT_SYMBOL_GPL(skb_gso_transport_seglen); > static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb) > { > if (skb_cow(skb, skb_headroom(skb)) < 0) { > - kfree_skb(skb); > return NULL; > } You now need to remove {}. MBR, Sergei
On Mon, 2016-02-29 at 12:22 +0000, Zhang Shengju wrote: > If skb_reorder_vlan_header() failed, skb is freed and NULL is returned. > Then at skb_vlan_untag(), it will free skbuff again which cause double > free. On skb_reorder_vlan_header() failure, skb_vlan_untag() will call kfree_skb() using the return value of skb_reorder_vlan_header(), that is NULL. kfree_skb() is a noop when the argument is NULL. The current code seams safe. Paolo
From: Zhang Shengju <zhangshengju@cmss.chinamobile.com> Date: Mon, 29 Feb 2016 12:22:53 +0000 > If skb_reorder_vlan_header() failed, skb is freed and NULL is returned. > Then at skb_vlan_untag(), it will free skbuff again which cause double > free. The 'skb' local variable in this case will be set to "NULL", calling kfree_skb() on NULL doesn't do anything. > This patch removes kfree_skb() call in function skb_reorder_vlan_header(). > > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> Please analyze the complete control path of the caller of this function, and you'll find that everything is fine.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 488566b..1312d4b 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4350,7 +4350,6 @@ EXPORT_SYMBOL_GPL(skb_gso_transport_seglen); static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb) { if (skb_cow(skb, skb_headroom(skb)) < 0) { - kfree_skb(skb); return NULL; }
If skb_reorder_vlan_header() failed, skb is freed and NULL is returned. Then at skb_vlan_untag(), it will free skbuff again which cause double free. This patch removes kfree_skb() call in function skb_reorder_vlan_header(). Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- net/core/skbuff.c | 1 - 1 file changed, 1 deletion(-)