Skip to contentSkip to conversation

If expecting pre-T11 frames, a T11 frame caused a data fault.

PATCH In review Joe Eykholt 2 participants 3 messages Dec 5, 2007 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 If expecting pre-T11 frames, a T11 frame caused a data fault.

If expecting pre-T11 frames, a T11 frame caused a data fault.

This is because the T11 frames have zeros where the old length/SOF field was, and a length of less than the size of the FC header was not checked for.

Also limit length error messages to frames (per-CPU actually).

---
 drivers/scsi/ofc/fcoe/fcoe_dev.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)
drivers/scsi/ofc/fcoe/fcoe_dev.c +15 -8
index df652b6..878efd4 100644
@@ -485,8 +485,9 @@ int fcoe_percpu_receive_thread(void *arg)
 
 			hp = (struct fcoe_hdr *)skb->data;
 			if (unlikely(FC_FCOE_DECAPS_VER(hp) != FC_FCOE_VER)) {
-				SA_LOG("unknown FCoE version %x",
-				       FC_FCOE_DECAPS_VER(hp));
+				if (stats->ErrorFrames < 5)
+					SA_LOG("unknown FCoE version %x",
+					       FC_FCOE_DECAPS_VER(hp));
 				stats->ErrorFrames++;
 				kfree_skb(skb);
 				continue;
@@ -505,20 +506,25 @@ int fcoe_percpu_receive_thread(void *arg)
 			skb_pull(skb, sizeof(*fchp));
 			fr_len = FC_FCOE_DECAPS_LEN(len);
 			fr_len = fr_len * FCOE_WORD_TO_BYTE;
+			if (unlikely(fr_len < sizeof(struct fc_frame_header) +
+			    sizeof(cp->fcoe_crc32))) {
+				if (stats->ErrorFrames < 5)
+					SA_LOG("length error: len_sof %x", len);
+				stats->ErrorFrames++;
+				kfree_skb(skb);
+				continue;
+			}
 			sof = FC_FCOE_DECAPS_SOF(len);
 			fr_len -= sizeof(cp->fcoe_crc32);
 			tlen = sizeof(struct fcoe_crc_eof_old);
 		}
 
 		if (skb_is_nonlinear(skb))
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,18)
-			skb_linearize(skb, GFP_KERNEL);	/* not ideal */
-#else
 			skb_linearize(skb);	/* not ideal */
-#endif
 		if (unlikely(fr_len + tlen > skb->len)) {
-			SA_LOG("short frame fr_len %x skb->len %x\n",
-			       fr_len + tlen, skb->len);
+			if (stats->ErrorFrames < 5)
+				SA_LOG("length error fr_len 0x%x skb->len 0x%x",
+				       fr_len + tlen, skb->len);
 			stats->ErrorFrames++;
 			kfree_skb(skb);
 			continue;
@@ -540,6 +546,7 @@ int fcoe_percpu_receive_thread(void *arg)
 		 * and it'll be more cache-efficient.
 		 */
 		fh = fc_frame_header_get(fp);
+		ASSERT(fh);
 		if (fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA &&
 		    fh->fh_type == FC_TYPE_FCP) {
 			fp->fr_flags |= FCPHF_CRC_UNCHECKED;
#1 lore raw

Just wondering:

Quoted 1 line

From my quick reading of the code these pre-T11 frames are basically

a different frame-level protocol. Given that T11 has standardized on a different one what's the rationale for supporting the old frames?

#2 lore raw
Quoted 5 lines · Christoph Hellwig

Just wondering:

Quoted 1 line

From my quick reading of the code these pre-T11 frames are basically

a different frame-level protocol. Given that T11 has standardized on a different one what's the rationale for supporting the old frames?

There is still some use in lab environments of the old frame format, and it would greatly help if we could keep it working for now. Eventually, in a year or so, it could be removed completely. It seems to be a small amount of code and in a loadable module besides.

Joe