netdev - Re: [PATCH] gianfar: fix potential sk_wmem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1341738542.3265.1406.camel@edumazet-glaptop>
Date:	Sun, 08 Jul 2012 11:09:02 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Paul Gortmaker <paul.gortmaker@...driver.com>
Cc:	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Manfred Rudigier <manfred.rudigier@...cron.at>,
	Claudiu Manoil <claudiu.manoil@...escale.com>,
	Jiajun Wu <b06378@...escale.com>,
	Andy Fleming <afleming@...escale.com>
Subject: Re: [PATCH] gianfar: fix potential sk_wmem_alloc imbalance

On Fri, 2012-07-06 at 14:09 -0400, Paul Gortmaker wrote:

> Aside from the one line change at driver init, is there more to it than
> that?  More specifically, it currently does:
> 
> fcb_length = GMAC_FCB_LEN;
> 
> if (...timestamps...)
> 	fcb_length = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
> 
> if (... && (skb_headroom(skb) < fcb_length))
> 	...
> 	skb_new = skb_realloc_headroom(skb, fcb_length);
> 
> and I don't know the code well enough to know if setting the
> needed_headroom value _guarantees_ the above fcb_length comparison
> will always be false, and hence can be deleted.  It kind of looks
> like it via LL_RESERVED_SPACE, but I'm not 100% sure...

This is not a guarantee but should take care of most cases.

So we should keep the test anyway.

And init needed_headroom to the largest room. Existing tests seems to
ignore vlan case and timestamping.

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index af16f9f..b4517b7 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1084,9 +1084,7 @@ static int gfar_probe(struct platform_device *ofdev)
 	else
 		priv->padding = 0;
 
-	if (dev->features & NETIF_F_IP_CSUM ||
-	    priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
-		dev->needed_headroom = GMAC_FCB_LEN;
+	dev->needed_headroom = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
 
 	/* Program the isrg regs only if number of grps > 1 */
 	if (priv->num_grps > 1) {



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html