Florian,

> -----Original Message-----
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: Monday, August 21, 2017 3:42 PM
> To: netdev@vger.kernel.org
> Cc: da...@davemloft.net; and...@lunn.ch;
> vivien.dide...@savoirfairelinux.com; Woojung Huh - C21699; Florian Fainelli
> Subject: [PATCH net] net: dsa: skb_put_padto() already frees nskb
> 
> skb_put_padto() already frees the passed sk_buff reference upon error,
> so calling kfree_skb() on it again is not necessary.
> 
> Detected by CoverityScan, CID#1416687 ("USE_AFTER_FREE")
> 
> Fixes: e71cb9e00922 ("net: dsa: ksz: fix skb freeing")
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
> ---
>  net/dsa/tag_ksz.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> index de66ca8e6201..107172c82107 100644
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -60,10 +60,8 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb,
> struct net_device *dev)
>                                        skb_transport_header(skb) - skb-
> >head);
>               skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
> 
> -             if (skb_put_padto(nskb, nskb->len + padlen)) {
> -                     kfree_skb(nskb);
> +             if (skb_put_padto(nskb, nskb->len + padlen))
>                       return NULL;
> -             }
> 
>               kfree_skb(skb);
>       }
> --

Because skb_put_padto() frees skb when it fails,  below lines in e71cb9e00922
("net: dsa: ksz: fix skb freeing") will be an issue to.

        if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
+               if (skb_put_padto(skb, skb->len + padlen))
+                       return NULL;
+

When it fails skb will be freed twice in skb_put_padto() and
caller of dsa_slave_xmit().

Woojung

Reply via email to