Florian,
> -----Original Message-----
> From: Florian Fainelli [mailto:[email protected]]
> Sent: Monday, August 21, 2017 3:42 PM
> To: [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; 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 <[email protected]>
> ---
> 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
