Patchwork net: fix skb_seq_read returning wrong offset/length for page frag data

login
register
mail settings
Submitter Thomas_Chenault@Dell.com
Date May 14, 2009, 1:33 a.m.
Message ID <65FDECA153DE9A40B78CA281949AC88902A9FBF1@ausx3mps309.aus.amer.dell.com>
Download mbox | patch
Permalink /patch/27186/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Thomas_Chenault@Dell.com - May 14, 2009, 1:33 a.m.
When called with a consumed value that is less than skb_headlen(skb)
bytes into a page frag, skb_seq_read() incorrectly returns an
offset/length relative to skb->data. Ensure that data which should come
from a page frag does.

Signed-off-by: Thomas Chenault <thomas_chenault@dell.com>

 		return block_limit - abs_offset;
 	}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shyam_Iyer@Dell.com - May 18, 2009, 1:02 p.m.
> -----Original Message-----
> From: Chenault, Thomas
> Sent: Thursday, May 14, 2009 7:03 AM
> To: 'netdev@vger.kernel.org'
> Cc: Iyer, Shyam; 'michaelc@cs.wisc.edu'
> Subject: [PATCH] net: fix skb_seq_read returning wrong offset/length
> for page frag data
> 
> When called with a consumed value that is less than skb_headlen(skb)
> bytes into a page frag, skb_seq_read() incorrectly returns an
> offset/length relative to skb->data. Ensure that data which should
come
> from a page frag does.
> 
> Signed-off-by: Thomas Chenault <thomas_chenault@dell.com>
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index d152394..e505b53 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2288,7 +2288,7 @@ unsigned int skb_seq_read(unsigned int consumed,
> const u8 **data,
>  next_skb:
>  	block_limit = skb_headlen(st->cur_skb) + st->stepped_offset;
> 
> -	if (abs_offset < block_limit) {
> +	if (abs_offset < block_limit && !st->frag_data) {
>  		*data = st->cur_skb->data + (abs_offset - st-
> >stepped_offset);
>  		return block_limit - abs_offset;
>  	}

Tested-by: Shyam Iyer <shyam_iyer@dell.com>

We have seen this scenario affect iSCSI transactions(mainly login) with
BCM57710 10G Ethernet(bnx2x driver) card.

The iSCSI login errs because the iSCSI PDU header is wrongly read with a
wrong ITT value. This error in reading the PDU is seen with this card
because of the way the bnx2x driver pushes data to the frags with LRO
enabled (through skb->frags[n]).

Intel 10G(ixgbe driver) + LRO combination does not expose the issue and
the patch doesn't cause a regression either. I am not sure if this patch
will break other driver+card configurations.  

Dave - Does this patch needs more review or you want to queue it up to
your tree? I believe it is a critical bug fix.

-Shyam Iyer
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - May 19, 2009, 2:40 a.m.
From: <Shyam_Iyer@Dell.com>
Date: Mon, 18 May 2009 18:32:29 +0530

> Dave - Does this patch needs more review or you want to queue it up to
> your tree? I believe it is a critical bug fix.

I just got back from a week of being away on a cruise, so don't
worry it's in my backlog.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - May 19, 2009, 4:44 a.m.
From: <Thomas_Chenault@Dell.com>
Date: Wed, 13 May 2009 20:33:11 -0500

> When called with a consumed value that is less than skb_headlen(skb)
> bytes into a page frag, skb_seq_read() incorrectly returns an
> offset/length relative to skb->data. Ensure that data which should come
> from a page frag does.
> 
> Signed-off-by: Thomas Chenault <thomas_chenault@dell.com>

Applied and queued up for -stable, thanks.

> @@ -2288,7 +2288,7 @@ unsigned int skb_seq_read(unsigned int consumed,
> const u8 **data,

Lots of lines in this patch were chopped up by your email client.
Please fix this up before you submit any future patches, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d152394..e505b53 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2288,7 +2288,7 @@  unsigned int skb_seq_read(unsigned int consumed,
const u8 **data,
 next_skb:
 	block_limit = skb_headlen(st->cur_skb) + st->stepped_offset;
 
-	if (abs_offset < block_limit) {
+	if (abs_offset < block_limit && !st->frag_data) {
 		*data = st->cur_skb->data + (abs_offset -
st->stepped_offset);