RE: [PATCH net] net: dsa: skb_put_padto() already frees nskb
flat view
From: <Woojung.Huh@microchip.com>
Date: 2017-08-21 22:16:12
Florian,
quoted hunk ↗ jump to hunk
-----Original Message----- From: Florian Fainelli [mailto:f.fainelli@gmail.com] Sent: Monday, August 21, 2017 3:42 PM To: netdev@vger.kernel.org Cc: davem@davemloft.net; andrew@lunn.ch; vivien.didelot@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.fainelli@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-quoted
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