[PATCH] net: return value of skb_linearize should be handled in Linux kernel

Subsystems: broadcom bnx2fc 10 gigabit fcoe driver, fcoe subsystem (libfc, libfcoe, fcoe), infiniband subsystem, intel ethernet drivers, networking drivers, networking [general], scsi subsystem, the rest, tipc network layer

STALE3560d

6 messages, 4 authors, 2016-12-07 · open the first message on its own page

[PATCH] net: return value of skb_linearize should be handled in Linux kernel

From: Zhouyi Zhou <hidden>
Date: 2016-12-06 07:10:54

kmalloc_reserve may fail to allocate memory inside skb_linearize, 
which means skb_linearize's return value should not be ignored. 
Following patch correct the uses of skb_linearize.

Compiled in x86_64

Signed-off-by: Zhouyi Zhou <redacted>
---
 drivers/infiniband/hw/nes/nes_nic.c           | 5 +++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c             | 7 +++++--
 drivers/scsi/fcoe/fcoe.c                      | 5 ++++-
 net/tipc/link.c                               | 3 ++-
 net/tipc/name_distr.c                         | 5 ++++-
 7 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 2b27d13..69372ea 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -662,10 +662,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, struct net_device *netdev)
 				nesnic->sq_head &= nesnic->sq_size-1;
 			}
 		} else {
-			nesvnic->linearized_skbs++;
 			hoffset = skb_transport_header(skb) - skb->data;
 			nhoffset = skb_network_header(skb) - skb->data;
-			skb_linearize(skb);
+			if (skb_linearize(skb))
+				return NETDEV_TX_BUSY;
+			nesvnic->linearized_skbs++;
 			skb_set_transport_header(skb, hoffset);
 			skb_set_network_header(skb, nhoffset);
 			if (!nes_nic_send(skb, netdev))
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 2a653ec..ab787cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
 	 */
 	if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
 	    (fctl & FC_FC_END_SEQ)) {
-		skb_linearize(skb);
+		int err = 0;
+
+		err = skb_linearize(skb);
+		if (err)
+			return err;
 		crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
 		crc->fcoe_eof = FC_EOF_T;
 	}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fee1f29..4926d48 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				total_rx_bytes += ddp_bytes;
 				total_rx_packets += DIV_ROUND_UP(ddp_bytes,
 								 mss);
-			}
-			if (!ddp_bytes) {
+			} else {
 				dev_kfree_skb_any(skb);
 				continue;
 			}
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index f9ddb61..197d02e 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -542,8 +542,11 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
 		return;
 	}
 
-	if (skb_is_nonlinear(skb))
-		skb_linearize(skb);
+	if (skb_linearize(skb)) {
+		kfree_skb(skb);
+		return;
+	}
+
 	mac = eth_hdr(skb)->h_source;
 	dest_mac = eth_hdr(skb)->h_dest;
 
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 9bd41a3..f691b97 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 			skb->dev ? skb->dev->name : "<NULL>");
 
 	port = lport_priv(lport);
-	skb_linearize(skb); /* check for skb_is_nonlinear is within skb_linearize */
+	if (skb_linearize(skb)) {
+		kfree_skb(skb);
+		return;
+	}
 
 	/*
 	 * Frame length checks and setting up the header pointers
diff --git a/net/tipc/link.c b/net/tipc/link.c
index bda89bf..077c570 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1446,7 +1446,8 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 	if (tipc_own_addr(l->net) > msg_prevnode(hdr))
 		l->net_plane = msg_net_plane(hdr);
 
-	skb_linearize(skb);
+	if (skb_linearize(skb))
+		goto exit;
 	hdr = buf_msg(skb);
 	data = msg_data(hdr);
 
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index c1cfd92..4e05d2a 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -356,7 +356,10 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *inputq)
 
 	spin_lock_bh(&tn->nametbl_lock);
 	for (skb = skb_dequeue(inputq); skb; skb = skb_dequeue(inputq)) {
-		skb_linearize(skb);
+		if (skb_linearize(skb)) {
+			kfree_skb(skb);
+			continue;
+		}
 		msg = buf_msg(skb);
 		mtype = msg_type(msg);
 		item = (struct distr_item *)msg_data(msg);
-- 
1.9.1

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

From: Yuval Shaia <hidden>
Date: 2016-12-06 16:57:59

On Tue, Dec 06, 2016 at 03:10:33PM +0800, Zhouyi Zhou wrote:
kmalloc_reserve may fail to allocate memory inside skb_linearize, 
which means skb_linearize's return value should not be ignored. 
Following patch correct the uses of skb_linearize.

Compiled in x86_64
FWIW compiled also on SPARC

Reviewed-by: Yuval Shaia <redacted>
quoted hunk
Signed-off-by: Zhouyi Zhou <redacted>
---
 drivers/infiniband/hw/nes/nes_nic.c           | 5 +++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c             | 7 +++++--
 drivers/scsi/fcoe/fcoe.c                      | 5 ++++-
 net/tipc/link.c                               | 3 ++-
 net/tipc/name_distr.c                         | 5 ++++-
 7 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 2b27d13..69372ea 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -662,10 +662,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, struct net_device *netdev)
 				nesnic->sq_head &= nesnic->sq_size-1;
 			}
 		} else {
-			nesvnic->linearized_skbs++;
 			hoffset = skb_transport_header(skb) - skb->data;
 			nhoffset = skb_network_header(skb) - skb->data;
-			skb_linearize(skb);
+			if (skb_linearize(skb))
+				return NETDEV_TX_BUSY;
+			nesvnic->linearized_skbs++;
 			skb_set_transport_header(skb, hoffset);
 			skb_set_network_header(skb, nhoffset);
 			if (!nes_nic_send(skb, netdev))
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 2a653ec..ab787cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
 	 */
 	if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
 	    (fctl & FC_FC_END_SEQ)) {
-		skb_linearize(skb);
+		int err = 0;
+
+		err = skb_linearize(skb);
+		if (err)
+			return err;
 		crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
 		crc->fcoe_eof = FC_EOF_T;
 	}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fee1f29..4926d48 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				total_rx_bytes += ddp_bytes;
 				total_rx_packets += DIV_ROUND_UP(ddp_bytes,
 								 mss);
-			}
-			if (!ddp_bytes) {
+			} else {
 				dev_kfree_skb_any(skb);
 				continue;
 			}
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index f9ddb61..197d02e 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -542,8 +542,11 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
 		return;
 	}
 
-	if (skb_is_nonlinear(skb))
-		skb_linearize(skb);
+	if (skb_linearize(skb)) {
+		kfree_skb(skb);
+		return;
+	}
+
 	mac = eth_hdr(skb)->h_source;
 	dest_mac = eth_hdr(skb)->h_dest;
 
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 9bd41a3..f691b97 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 			skb->dev ? skb->dev->name : "<NULL>");
 
 	port = lport_priv(lport);
-	skb_linearize(skb); /* check for skb_is_nonlinear is within skb_linearize */
+	if (skb_linearize(skb)) {
+		kfree_skb(skb);
+		return;
+	}
 
 	/*
 	 * Frame length checks and setting up the header pointers
diff --git a/net/tipc/link.c b/net/tipc/link.c
index bda89bf..077c570 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1446,7 +1446,8 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 	if (tipc_own_addr(l->net) > msg_prevnode(hdr))
 		l->net_plane = msg_net_plane(hdr);
 
-	skb_linearize(skb);
+	if (skb_linearize(skb))
+		goto exit;
 	hdr = buf_msg(skb);
 	data = msg_data(hdr);
 
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index c1cfd92..4e05d2a 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -356,7 +356,10 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *inputq)
 
 	spin_lock_bh(&tn->nametbl_lock);
 	for (skb = skb_dequeue(inputq); skb; skb = skb_dequeue(inputq)) {
-		skb_linearize(skb);
+		if (skb_linearize(skb)) {
+			kfree_skb(skb);
+			continue;
+		}
 		msg = buf_msg(skb);
 		mtype = msg_type(msg);
 		item = (struct distr_item *)msg_data(msg);
-- 
1.9.1

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

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

From: Cong Wang <hidden>
Date: 2016-12-07 05:02:26

On Mon, Dec 5, 2016 at 11:10 PM, Zhouyi Zhou [off-list ref] wrote:
quoted hunk
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 2a653ec..ab787cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
         */
        if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
            (fctl & FC_FC_END_SEQ)) {
-               skb_linearize(skb);
+               int err = 0;
+
+               err = skb_linearize(skb);
+               if (err)
+                       return err;

You can reuse 'rc' instead of adding 'err'.


quoted hunk
                crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
                crc->fcoe_eof = FC_EOF_T;
        }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fee1f29..4926d48 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
                                total_rx_bytes += ddp_bytes;
                                total_rx_packets += DIV_ROUND_UP(ddp_bytes,
                                                                 mss);
-                       }
-                       if (!ddp_bytes) {
+                       } else {
                                dev_kfree_skb_any(skb);
                                continue;
                        }

This piece doesn't seem to be related.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

From: Zhouyi Zhou <hidden>
Date: 2016-12-07 06:27:09

On Wed, Dec 7, 2016 at 1:02 PM, Cong Wang [off-list ref] wrote:
On Mon, Dec 5, 2016 at 11:10 PM, Zhouyi Zhou [off-list ref] wrote:
quoted
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 2a653ec..ab787cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
         */
        if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
            (fctl & FC_FC_END_SEQ)) {
-               skb_linearize(skb);
+               int err = 0;
+
+               err = skb_linearize(skb);
+               if (err)
+                       return err;

You can reuse 'rc' instead of adding 'err'.
rc here is meaningful for the length of data being ddped. If using rc
here, a successful
skb_linearize will assign rc to 0.

quoted
                crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
                crc->fcoe_eof = FC_EOF_T;
        }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fee1f29..4926d48 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
                                total_rx_bytes += ddp_bytes;
                                total_rx_packets += DIV_ROUND_UP(ddp_bytes,
                                                                 mss);
-                       }
-                       if (!ddp_bytes) {
+                       } else {
                                dev_kfree_skb_any(skb);
                                continue;
                        }

This piece doesn't seem to be related.
if ddp_bytes is negative there will be some error, I think the skb
should not pass to upper layer.

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

From: Eric Dumazet <hidden>
Date: 2016-12-07 06:51:29

On Tue, 2016-12-06 at 15:10 +0800, Zhouyi Zhou wrote:
quoted hunk
kmalloc_reserve may fail to allocate memory inside skb_linearize, 
which means skb_linearize's return value should not be ignored. 
Following patch correct the uses of skb_linearize.

Compiled in x86_64

Signed-off-by: Zhouyi Zhou <redacted>
---
 drivers/infiniband/hw/nes/nes_nic.c           | 5 +++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c             | 7 +++++--
 drivers/scsi/fcoe/fcoe.c                      | 5 ++++-
 net/tipc/link.c                               | 3 ++-
 net/tipc/name_distr.c                         | 5 ++++-
 7 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 2b27d13..69372ea 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -662,10 +662,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, struct net_device *netdev)
 				nesnic->sq_head &= nesnic->sq_size-1;
 			}
 		} else {
-			nesvnic->linearized_skbs++;
 			hoffset = skb_transport_header(skb) - skb->data;
 			nhoffset = skb_network_header(skb) - skb->data;
-			skb_linearize(skb);
+			if (skb_linearize(skb))
+				return NETDEV_TX_BUSY;
This would live lock.

Please drop the packet.

You probably should send one patch per driver, to ease code review and
acceptance.

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

From: Cong Wang <hidden>
Date: 2016-12-07 17:58:12

On Tue, Dec 6, 2016 at 10:27 PM, Zhouyi Zhou [off-list ref] wrote:
On Wed, Dec 7, 2016 at 1:02 PM, Cong Wang [off-list ref] wrote:
quoted
On Mon, Dec 5, 2016 at 11:10 PM, Zhouyi Zhou [off-list ref] wrote:
quoted
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 2a653ec..ab787cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
         */
        if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
            (fctl & FC_FC_END_SEQ)) {
-               skb_linearize(skb);
+               int err = 0;
+
+               err = skb_linearize(skb);
+               if (err)
+                       return err;

You can reuse 'rc' instead of adding 'err'.
rc here is meaningful for the length of data being ddped. If using rc
here, a successful
skb_linearize will assign rc to 0.
Right, I thought it returns 0 on success.

quoted

quoted
                crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
                crc->fcoe_eof = FC_EOF_T;
        }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fee1f29..4926d48 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
                                total_rx_bytes += ddp_bytes;
                                total_rx_packets += DIV_ROUND_UP(ddp_bytes,
                                                                 mss);
-                       }
-                       if (!ddp_bytes) {
+                       } else {
                                dev_kfree_skb_any(skb);
                                continue;
                        }

This piece doesn't seem to be related.
if ddp_bytes is negative there will be some error, I think the skb
should not pass to upper layer.
You misunderstand my point, this return value is for ixgbe_fcoe_ddp()
not skb_linearize(), you need to make it a separate patch because this
patch, as in $subject, only fixes skb_linearize().
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help