All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Love <robert.w.love@intel.com>
To: James.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org
Cc: Chris Leech <christopher.leech@intel.com>,
	Robert Love <robert.w.love@intel.com>
Subject: [PATCH 09/35] libfc, fcoe: fixes for highmem skb linearize panics
Date: Fri, 11 Sep 2009 16:57:43 -0700	[thread overview]
Message-ID: <20090911235743.27223.96563.stgit@localhost.localdomain> (raw)
In-Reply-To: <20090911235655.27223.69728.stgit@localhost.localdomain>

From: Chris Leech <christopher.leech@intel.com>

There are cases outside of our control that may result in a transmit
skb being linearized in dev_queue_xmit.  There are a couple of bugs
in libfc/fcoe that can result in a panic at that point.  This patch
contains two fixes to prevent those panics.

1) use fast cloning instead of shared skbs with dev_queue_xmit

dev_queue_xmit doen't want shared skbuffs being passed in, and
__skb_linearize will BUG if the skb is shared.  FCoE is holding an extra
reference around the call to dev_queue_xmit, so that when it returns an
error code indicating the frame has been dropped it can maintain it's
own backlog and retransmit.  Switch to using fast skb cloning for this
instead.

2) don't append compound pages as > PAGE_SIZE skb fragments

fc_fcp_send_data will append pages from a scatterlist to the nr_frags[]
if the netdev supports it.  But, it's using > PAGE_SIZE compound pages
as a single skb_frag.  In the highmem linearize case that page will be
passed to kmap_atomic to get a mapping to copy out of, but
kmap_atomic will only allow access to the first PAGE_SIZE part.
The memcpy will keep going and cause a page fault once is crosses the
first boundary.

If fc_fcp_send_data uses linear buffers from the start, it calls
kmap_atomic one PAGE_SIZE at a time.  That same logic needs to be
applied when setting up skb_frags.

Signed-off-by: Chris Leech <christopher.leech@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/fcoe.c      |    5 +++--
 drivers/scsi/libfc/fc_fcp.c   |   20 ++++++++++----------
 drivers/scsi/libfc/fc_frame.c |    5 +++--
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index c8dd228..6926ed7 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1084,10 +1084,11 @@ err2:
  */
 static inline int fcoe_start_io(struct sk_buff *skb)
 {
+	struct sk_buff *nskb;
 	int rc;
 
-	skb_get(skb);
-	rc = dev_queue_xmit(skb);
+	nskb = skb_clone(skb, GFP_ATOMIC);
+	rc = dev_queue_xmit(nskb);
 	if (rc != 0)
 		return rc;
 	kfree_skb(skb);
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index aa3eafd..e4ff27e 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -455,11 +455,13 @@ static int fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *seq,
 	struct scatterlist *sg;
 	struct fc_frame *fp = NULL;
 	struct fc_lport *lp = fsp->lp;
+	struct page *page;
 	size_t remaining;
 	size_t t_blen;
 	size_t tlen;
 	size_t sg_bytes;
 	size_t frame_offset, fh_parm_offset;
+	size_t off;
 	int error;
 	void *data = NULL;
 	void *page_addr;
@@ -537,28 +539,26 @@ static int fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *seq,
 			fh_parm_offset = frame_offset;
 			fr_max_payload(fp) = fsp->max_payload;
 		}
+
+		off = offset + sg->offset;
 		sg_bytes = min(tlen, sg->length - offset);
+		sg_bytes = min(sg_bytes,
+			       (size_t) (PAGE_SIZE - (off & ~PAGE_MASK)));
+		page = sg_page(sg) + (off >> PAGE_SHIFT);
 		if (using_sg) {
-			get_page(sg_page(sg));
+			get_page(page);
 			skb_fill_page_desc(fp_skb(fp),
 					   skb_shinfo(fp_skb(fp))->nr_frags,
-					   sg_page(sg), sg->offset + offset,
-					   sg_bytes);
+					   page, off & ~PAGE_MASK, sg_bytes);
 			fp_skb(fp)->data_len += sg_bytes;
 			fr_len(fp) += sg_bytes;
 			fp_skb(fp)->truesize += PAGE_SIZE;
 		} else {
-			size_t off = offset + sg->offset;
-
 			/*
 			 * The scatterlist item may be bigger than PAGE_SIZE,
 			 * but we must not cross pages inside the kmap.
 			 */
-			sg_bytes = min(sg_bytes, (size_t) (PAGE_SIZE -
-							   (off & ~PAGE_MASK)));
-			page_addr = kmap_atomic(sg_page(sg) +
-						(off >> PAGE_SHIFT),
-						KM_SOFTIRQ0);
+			page_addr = kmap_atomic(page, KM_SOFTIRQ0);
 			memcpy(data, (char *)page_addr + (off & ~PAGE_MASK),
 			       sg_bytes);
 			kunmap_atomic(page_addr, KM_SOFTIRQ0);
diff --git a/drivers/scsi/libfc/fc_frame.c b/drivers/scsi/libfc/fc_frame.c
index 63fe00c..5b9c977 100644
--- a/drivers/scsi/libfc/fc_frame.c
+++ b/drivers/scsi/libfc/fc_frame.c
@@ -58,12 +58,13 @@ struct fc_frame *__fc_frame_alloc(size_t len)
 
 	WARN_ON((len % sizeof(u32)) != 0);
 	len += sizeof(struct fc_frame_header);
-	skb = dev_alloc_skb(len + FC_FRAME_HEADROOM + FC_FRAME_TAILROOM);
+	skb = alloc_skb_fclone(len + FC_FRAME_HEADROOM + FC_FRAME_TAILROOM +
+			       NET_SKB_PAD, GFP_ATOMIC);
 	if (!skb)
 		return NULL;
+	skb_reserve(skb, NET_SKB_PAD + FC_FRAME_HEADROOM);
 	fp = (struct fc_frame *) skb;
 	fc_frame_init(fp);
-	skb_reserve(skb, FC_FRAME_HEADROOM);
 	skb_put(skb, len);
 	return fp;
 }


  parent reply	other threads:[~2009-09-11 23:58 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-11 23:56 [PATCH 00/35] libfc, libfcoe and fcoe updates for 2.6.32 Robert Love
2009-09-11 23:57 ` [PATCH 01/35] libfc: fix typo in retry check on received PRLI Robert Love
2009-09-11 23:57 ` [PATCH 02/35] fcoe: Increase FCOE_MAX_LUN to 0xFFFF (65535) Robert Love
2009-09-11 23:57 ` [PATCH 03/35] libfc: Move non-common routines and prototypes out of libfc.h Robert Love
2009-09-11 23:57 ` [PATCH 04/35] libfc: Remove fc_fcp_complete Robert Love
2009-09-11 23:57 ` [PATCH 05/35] libfc: Add libfc/fc_libfc.[ch] for libfc internal routines Robert Love
2009-09-11 23:57 ` [PATCH 06/35] libfc: Move libfc_init and libfc_exit to fc_libfc.c Robert Love
2009-09-11 23:57 ` [PATCH 07/35] libfc: fix ddp in fc_fcp for 0 xid Robert Love
2009-09-11 23:57 ` [PATCH 08/35] fcoe: remove redundant checking of netdev->netdev_ops Robert Love
2009-09-11 23:57 ` Robert Love [this message]
2009-09-11 23:57 ` [PATCH 10/35] libfc: changes to libfc_host_alloc to consolidate initialization with allocation Robert Love
2009-09-11 23:57 ` [PATCH 11/35] libfc: add some generic NPIV support routines to libfc Robert Love
2009-09-11 23:57 ` [PATCH 12/35] libfc: vport link handling and fc_vport state managment Robert Love
2009-09-11 23:58 ` [PATCH 13/35] libfc, libfcoe: FDISC ELS for NPIV Robert Love
2009-09-11 23:58 ` [PATCH 14/35] libfcoe, fcoe: libfcoe NPIV support Robert Love
2009-09-11 23:58 ` [PATCH 15/35] fcoe: add a separate scsi transport template for NPIV vports Robert Love
2009-09-11 23:58 ` [PATCH 16/35] fcoe: NPIV vport create/destroy Robert Love
2009-09-11 23:58 ` [PATCH 17/35] libfc: RPN_ID is obsolete and unnecessary Robert Love
2009-09-11 23:58 ` [PATCH 18/35] libfc: RNN_ID may be required before RSNN_NN with some switches Robert Love
2009-09-11 23:58 ` [PATCH 19/35] libfc: Register Symbolic Node Name (RSNN_NN) Robert Love
2009-09-11 23:58 ` [PATCH 20/35] libfc: Register Symbolic Port Name (RSPN_ID) Robert Love
2009-09-11 23:58 ` [PATCH 21/35] libfc: combine name server registration response handlers Robert Love
2009-09-11 23:58 ` [PATCH 22/35] libfc: combine name server registration request functions Robert Love
2009-09-11 23:58 ` [PATCH 23/35] fcoe: vport symbolic name support Robert Love
2009-09-11 23:59 ` [PATCH 24/35] libfc: Export FC headers Robert Love
2009-09-11 23:59 ` [PATCH 25/35] libfc: Add routine to copy data from a buffer to a SG list Robert Love
2009-09-11 23:59 ` [PATCH 26/35] libfc, fcoe: Add FC passthrough support Robert Love
2009-09-11 23:59 ` [PATCH 27/35] libfc, fcoe: Don't EXPORT_SYMBOLS unnecessarily Robert Love
2009-09-11 23:59 ` [PATCH 28/35] libfc: Remove unused fc_lport pointer from fc_fcp_pkt_abort Robert Love
2009-09-11 23:59 ` [PATCH 29/35] libfc: Formatting cleanups across libfc Robert Love
2009-09-11 23:59 ` [PATCH 30/35] libfcoe: formatting and comment cleanups Robert Love
2009-09-11 23:59 ` [PATCH 31/35] fcoe: Formatting cleanups and commenting Robert Love
2009-09-11 23:59 ` [PATCH 32/35] libfc: Fix wrong scsi return status under FC_DATA_UNDRUN Robert Love
2009-09-11 23:59 ` [PATCH 33/35] fcoe, libfc: use single frame allocation API Robert Love
2009-09-12  0:00 ` [PATCH 34/35] libfc: reduce can_queue for all FCP frame allocation failures Robert Love
2009-09-12  0:00 ` [PATCH 35/35] libfc: adds can_queue ramp up Robert Love
2009-09-14 17:18   ` Mike Christie
2009-09-14 23:23     ` Vasu Dev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090911235743.27223.96563.stgit@localhost.localdomain \
    --to=robert.w.love@intel.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=christopher.leech@intel.com \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.