| Message ID | 200901270512.n0R5CLXB019113@darkside.asicdesigners.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Dimitris Michailidis <dm@chelsio.com> Date: Mon, 26 Jan 2009 21:12:21 -0800 > Fix length tcp_splice_data_recv passes to skb_splice_bits. Eric, please review this. -- 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
CCed Willy Tarreau Dimitris Michailidis a écrit : > commit 6c242233648471868b44ea091d461f2db6a93f10 > Author: Dimitris Michailidis <dm@chelsio.com> > Date: Mon Jan 26 20:46:56 2009 -0800 > > Fix length tcp_splice_data_recv passes to skb_splice_bits. > > tcp_splice_data_recv has two lengths to consider: the len parameter it > gets from tcp_read_sock, which specifies the amount of data in the skb, > and rd_desc->count, which is the amount of data the splice caller still > wants. Currently it passes just the latter to skb_splice_bits, which then > splices min(rd_desc->count, skb->len - offset) bytes. > > Most of the time this is fine, except when the skb contains urgent data. > In that case len goes only up to the urgent byte and is less than > skb->len - offset. By ignoring len tcp_splice_data_recv may a) splice > data tcp_read_sock told it not to, b) return to tcp_read_sock a value > len. > > Now, tcp_read_sock doesn't handle used > len and leaves the socket in a > bad state (both sk_receive_queue and copied_seq are bad at that point) > resulting in duplicated data and corruption. > > Fix by passing min(rd_desc->count, len) to skb_splice_bits. > > Signed-off-by: Dimitris Michailidis <dm@chelsio.com> > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 0cd71b8..76b148b 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -524,7 +524,8 @@ static int tcp_splice_data_recv(read_descriptor_t *rd_desc, struct sk_buff *skb, > struct tcp_splice_state *tss = rd_desc->arg.data; > int ret; > > - ret = skb_splice_bits(skb, offset, tss->pipe, rd_desc->count, tss->flags); > + ret = skb_splice_bits(skb, offset, tss->pipe, min(rd_desc->count, len), > + tss->flags); > if (ret > 0) > rd_desc->count -= ret; > return ret; Nice spot Dimitris ! Acked-by: Eric Dumazet <dada1@cosmosbay.com> This fixes a bug present in previous linux versions (before commit 33966dd0e2f68f26943cd9ee93ec6abbc6547a8e tcp: splice as many packets as possible at once) It should be backported as well, changing tss->len by min(tss->len, len) ? -- 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
From: Eric Dumazet <dada1@cosmosbay.com> Date: Tue, 27 Jan 2009 07:10:34 +0100 > Nice spot Dimitris ! > > Acked-by: Eric Dumazet <dada1@cosmosbay.com> Ok, I'll apply this. > This fixes a bug present in previous linux versions (before commit > 33966dd0e2f68f26943cd9ee93ec6abbc6547a8e tcp: splice as many packets > as possible at once) > > It should be backported as well, changing tss->len by min(tss->len, len) ? Indeed, I'll queue this up for -stable to remind myself about this. Thanks Eric. -- 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
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 0cd71b8..76b148b 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -524,7 +524,8 @@ static int tcp_splice_data_recv(read_descriptor_t *rd_desc, struct sk_buff *skb, struct tcp_splice_state *tss = rd_desc->arg.data; int ret; - ret = skb_splice_bits(skb, offset, tss->pipe, rd_desc->count, tss->flags); + ret = skb_splice_bits(skb, offset, tss->pipe, min(rd_desc->count, len), + tss->flags); if (ret > 0) rd_desc->count -= ret; return ret;
commit 6c242233648471868b44ea091d461f2db6a93f10 Author: Dimitris Michailidis <dm@chelsio.com> Date: Mon Jan 26 20:46:56 2009 -0800 Fix length tcp_splice_data_recv passes to skb_splice_bits. tcp_splice_data_recv has two lengths to consider: the len parameter it gets from tcp_read_sock, which specifies the amount of data in the skb, and rd_desc->count, which is the amount of data the splice caller still wants. Currently it passes just the latter to skb_splice_bits, which then splices min(rd_desc->count, skb->len - offset) bytes. Most of the time this is fine, except when the skb contains urgent data. In that case len goes only up to the urgent byte and is less than skb->len - offset. By ignoring len tcp_splice_data_recv may a) splice data tcp_read_sock told it not to, b) return to tcp_read_sock a value > len. Now, tcp_read_sock doesn't handle used > len and leaves the socket in a bad state (both sk_receive_queue and copied_seq are bad at that point) resulting in duplicated data and corruption. Fix by passing min(rd_desc->count, len) to skb_splice_bits. Signed-off-by: Dimitris Michailidis <dm@chelsio.com> -- 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