netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs
@ 2017-04-19 16:59 Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 1/6] smsc75xx: " Eric Dumazet
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet

James Hughes found an issue with smsc95xx driver. Same problematic code
is found in other drivers.

Eric Dumazet (6):
  smsc75xx: use skb_cow_head() to deal with cloned skbs
  cx82310_eth: use skb_cow_head() to deal with cloned skbs
  sr9700: use skb_cow_head() to deal with cloned skbs
  lan78xx: use skb_cow_head() to deal with cloned skbs
  ch9200: use skb_cow_head() to deal with cloned skbs
  kaweth: use skb_cow_head() to deal with cloned skbs

 drivers/net/usb/ch9200.c      |  9 ++-------
 drivers/net/usb/cx82310_eth.c |  7 ++-----
 drivers/net/usb/kaweth.c      | 18 ++++++------------
 drivers/net/usb/lan78xx.c     |  9 ++-------
 drivers/net/usb/smsc75xx.c    |  8 ++------
 drivers/net/usb/sr9700.c      |  9 ++-------
 6 files changed, 16 insertions(+), 44 deletions(-)

-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net 1/6] smsc75xx: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
@ 2017-04-19 16:59 ` Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 2/6] cx82310_eth: " Eric Dumazet
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet

We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.

skb_cow_head() is the proper helper to deal with this.

Fixes: d0cad871703b ("smsc75xx: SMSC LAN75xx USB gigabit ethernet adapter driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
---
 drivers/net/usb/smsc75xx.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 0b17b40d7a4fa2653caf21406c4a6b3b45d868b0..190de9a90f7387c5070c7f589aa18bb7d05ac5d7 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2203,13 +2203,9 @@ static struct sk_buff *smsc75xx_tx_fixup(struct usbnet *dev,
 {
 	u32 tx_cmd_a, tx_cmd_b;
 
-	if (skb_headroom(skb) < SMSC75XX_TX_OVERHEAD) {
-		struct sk_buff *skb2 =
-			skb_copy_expand(skb, SMSC75XX_TX_OVERHEAD, 0, flags);
+	if (skb_cow_head(skb, SMSC75XX_TX_OVERHEAD)) {
 		dev_kfree_skb_any(skb);
-		skb = skb2;
-		if (!skb)
-			return NULL;
+		return NULL;
 	}
 
 	tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN) | TX_CMD_A_FCS;
-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 2/6] cx82310_eth: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 1/6] smsc75xx: " Eric Dumazet
@ 2017-04-19 16:59 ` Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 3/6] sr9700: " Eric Dumazet
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet

We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.

skb_cow_head() is the proper helper to deal with this.

Fixes: cc28a20e77b2 ("introduce cx82310_eth: Conexant CX82310-based ADSL router USB ethernet driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
---
 drivers/net/usb/cx82310_eth.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/cx82310_eth.c b/drivers/net/usb/cx82310_eth.c
index e221bfcee76b40a3ad7ba60ec4d348f4b8f4cc73..947bea81d924124c3827e87f75e732e35adb2acd 100644
--- a/drivers/net/usb/cx82310_eth.c
+++ b/drivers/net/usb/cx82310_eth.c
@@ -293,12 +293,9 @@ static struct sk_buff *cx82310_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 {
 	int len = skb->len;
 
-	if (skb_headroom(skb) < 2) {
-		struct sk_buff *skb2 = skb_copy_expand(skb, 2, 0, flags);
+	if (skb_cow_head(skb, 2)) {
 		dev_kfree_skb_any(skb);
-		skb = skb2;
-		if (!skb)
-			return NULL;
+		return NULL;
 	}
 	skb_push(skb, 2);
 
-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 3/6] sr9700: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 1/6] smsc75xx: " Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 2/6] cx82310_eth: " Eric Dumazet
@ 2017-04-19 16:59 ` Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 4/6] lan78xx: " Eric Dumazet
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet

We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.

skb_cow_head() is the proper helper to deal with this.

Fixes: c9b37458e956 ("USB2NET : SR9700 : One chip USB 1.1 USB2NET SR9700Device Driver Support")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
---
 drivers/net/usb/sr9700.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 4a1e9c489f1f455388ffee289d65e1d6b36cba42..aadfe1d1c37ee67e2a7d17c759db12dad248c41d 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -456,14 +456,9 @@ static struct sk_buff *sr9700_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 
 	len = skb->len;
 
-	if (skb_headroom(skb) < SR_TX_OVERHEAD) {
-		struct sk_buff *skb2;
-
-		skb2 = skb_copy_expand(skb, SR_TX_OVERHEAD, 0, flags);
+	if (skb_cow_head(skb, SR_TX_OVERHEAD)) {
 		dev_kfree_skb_any(skb);
-		skb = skb2;
-		if (!skb)
-			return NULL;
+		return NULL;
 	}
 
 	__skb_push(skb, SR_TX_OVERHEAD);
-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 4/6] lan78xx: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
                   ` (2 preceding siblings ...)
  2017-04-19 16:59 ` [PATCH net 3/6] sr9700: " Eric Dumazet
@ 2017-04-19 16:59 ` Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 5/6] ch9200: " Eric Dumazet
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet, Woojung Huh

We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.

skb_cow_head() is the proper helper to deal with this.

Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
Cc: Woojung Huh <woojung.huh@microchip.com>
---
 drivers/net/usb/lan78xx.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 9889a70ff4f6fece5bfabbfb45a3470f721a5a32..636f48f19d1eacae67c050de4fc3e651bffdf825 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2607,14 +2607,9 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
 {
 	u32 tx_cmd_a, tx_cmd_b;
 
-	if (skb_headroom(skb) < TX_OVERHEAD) {
-		struct sk_buff *skb2;
-
-		skb2 = skb_copy_expand(skb, TX_OVERHEAD, 0, flags);
+	if (skb_cow_head(skb, TX_OVERHEAD)) {
 		dev_kfree_skb_any(skb);
-		skb = skb2;
-		if (!skb)
-			return NULL;
+		return NULL;
 	}
 
 	if (lan78xx_linearize(skb) < 0)
-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 5/6] ch9200: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
                   ` (3 preceding siblings ...)
  2017-04-19 16:59 ` [PATCH net 4/6] lan78xx: " Eric Dumazet
@ 2017-04-19 16:59 ` Eric Dumazet
  2017-04-19 16:59 ` [PATCH net 6/6] kaweth: " Eric Dumazet
  2017-04-21 17:25 ` [PATCH net 0/6] net: " David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet, Matthew Garrett

We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.

skb_cow_head() is the proper helper to deal with this.

Fixes: 4a476bd6d1d9 ("usbnet: New driver for QinHeng CH9200 devices")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
---
 drivers/net/usb/ch9200.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/ch9200.c b/drivers/net/usb/ch9200.c
index 8a40202c0a1732850da1b2eb64af21470b9c85b4..c4f1c363e24b89404c6834312074f8a4451ded50 100644
--- a/drivers/net/usb/ch9200.c
+++ b/drivers/net/usb/ch9200.c
@@ -254,14 +254,9 @@ static struct sk_buff *ch9200_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 	tx_overhead = 0x40;
 
 	len = skb->len;
-	if (skb_headroom(skb) < tx_overhead) {
-		struct sk_buff *skb2;
-
-		skb2 = skb_copy_expand(skb, tx_overhead, 0, flags);
+	if (skb_cow_head(skb, tx_overhead)) {
 		dev_kfree_skb_any(skb);
-		skb = skb2;
-		if (!skb)
-			return NULL;
+		return NULL;
 	}
 
 	__skb_push(skb, tx_overhead);
-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 6/6] kaweth: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
                   ` (4 preceding siblings ...)
  2017-04-19 16:59 ` [PATCH net 5/6] ch9200: " Eric Dumazet
@ 2017-04-19 16:59 ` Eric Dumazet
  2017-04-21 17:25 ` [PATCH net 0/6] net: " David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet

We can use skb_cow_head() to properly deal with clones,
especially the ones coming from TCP stack that allow their head being
modified. This avoids a copy.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
---
 drivers/net/usb/kaweth.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index 876f02f4945eafdc2fb5cfa0f9dcb54d9b498af4..2a2c3edb6bad0b3bd257c3a101d100ad3b00cc59 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -803,18 +803,12 @@ static netdev_tx_t kaweth_start_xmit(struct sk_buff *skb,
 	}
 
 	/* We now decide whether we can put our special header into the sk_buff */
-	if (skb_cloned(skb) || skb_headroom(skb) < 2) {
-		/* no such luck - we make our own */
-		struct sk_buff *copied_skb;
-		copied_skb = skb_copy_expand(skb, 2, 0, GFP_ATOMIC);
-		dev_kfree_skb_irq(skb);
-		skb = copied_skb;
-		if (!copied_skb) {
-			kaweth->stats.tx_errors++;
-			netif_start_queue(net);
-			spin_unlock_irq(&kaweth->device_lock);
-			return NETDEV_TX_OK;
-		}
+	if (skb_cow_head(skb, 2)) {
+		kaweth->stats.tx_errors++;
+		netif_start_queue(net);
+		spin_unlock_irq(&kaweth->device_lock);
+		dev_kfree_skb_any(skb);
+		return NETDEV_TX_OK;
 	}
 
 	private_header = (__le16 *)__skb_push(skb, 2);
-- 
2.12.2.816.g2cccc81164-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs
  2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
                   ` (5 preceding siblings ...)
  2017-04-19 16:59 ` [PATCH net 6/6] kaweth: " Eric Dumazet
@ 2017-04-21 17:25 ` David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2017-04-21 17:25 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, james.hughes, eric.dumazet

From: Eric Dumazet <edumazet@google.com>
Date: Wed, 19 Apr 2017 09:59:20 -0700

> James Hughes found an issue with smsc95xx driver. Same problematic code
> is found in other drivers.

Series applied, thanks Eric.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-04-21 18:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-19 16:59 [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs Eric Dumazet
2017-04-19 16:59 ` [PATCH net 1/6] smsc75xx: " Eric Dumazet
2017-04-19 16:59 ` [PATCH net 2/6] cx82310_eth: " Eric Dumazet
2017-04-19 16:59 ` [PATCH net 3/6] sr9700: " Eric Dumazet
2017-04-19 16:59 ` [PATCH net 4/6] lan78xx: " Eric Dumazet
2017-04-19 16:59 ` [PATCH net 5/6] ch9200: " Eric Dumazet
2017-04-19 16:59 ` [PATCH net 6/6] kaweth: " Eric Dumazet
2017-04-21 17:25 ` [PATCH net 0/6] net: " David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).