| From 4f25abff83e2780265eaa17d437b7659ea543bd5 Mon Sep 17 00:00:00 2001 |
| From: Eric Dumazet <edumazet@google.com> |
| Date: Tue, 15 Oct 2013 11:54:30 -0700 |
| Subject: tcp: must unclone packets before mangling them |
| |
| From: Eric Dumazet <edumazet@google.com> |
| |
| [ Upstream commit c52e2421f7368fd36cbe330d2cf41b10452e39a9 ] |
| |
| TCP stack should make sure it owns skbs before mangling them. |
| |
| We had various crashes using bnx2x, and it turned out gso_size |
| was cleared right before bnx2x driver was populating TC descriptor |
| of the _previous_ packet send. TCP stack can sometime retransmit |
| packets that are still in Qdisc. |
| |
| Of course we could make bnx2x driver more robust (using |
| ACCESS_ONCE(shinfo->gso_size) for example), but the bug is TCP stack. |
| |
| We have identified two points where skb_unclone() was needed. |
| |
| This patch adds a WARN_ON_ONCE() to warn us if we missed another |
| fix of this kind. |
| |
| Kudos to Neal for finding the root cause of this bug. Its visible |
| using small MSS. |
| |
| Signed-off-by: Eric Dumazet <edumazet@google.com> |
| Signed-off-by: Neal Cardwell <ncardwell@google.com> |
| Cc: Yuchung Cheng <ycheng@google.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/ipv4/tcp_output.c | 9 ++++++--- |
| 1 file changed, 6 insertions(+), 3 deletions(-) |
| |
| --- a/net/ipv4/tcp_output.c |
| +++ b/net/ipv4/tcp_output.c |
| @@ -981,6 +981,9 @@ static void tcp_queue_skb(struct sock *s |
| static void tcp_set_skb_tso_segs(const struct sock *sk, struct sk_buff *skb, |
| unsigned int mss_now) |
| { |
| + /* Make sure we own this skb before messing gso_size/gso_segs */ |
| + WARN_ON_ONCE(skb_cloned(skb)); |
| + |
| if (skb->len <= mss_now || !sk_can_gso(sk) || |
| skb->ip_summed == CHECKSUM_NONE) { |
| /* Avoid the costly divide in the normal |
| @@ -1062,9 +1065,7 @@ int tcp_fragment(struct sock *sk, struct |
| if (nsize < 0) |
| nsize = 0; |
| |
| - if (skb_cloned(skb) && |
| - skb_is_nonlinear(skb) && |
| - pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) |
| + if (skb_unclone(skb, GFP_ATOMIC)) |
| return -ENOMEM; |
| |
| /* Get a new skb... force flag on. */ |
| @@ -2339,6 +2340,8 @@ int __tcp_retransmit_skb(struct sock *sk |
| int oldpcount = tcp_skb_pcount(skb); |
| |
| if (unlikely(oldpcount > 1)) { |
| + if (skb_unclone(skb, GFP_ATOMIC)) |
| + return -ENOMEM; |
| tcp_init_tso_segs(sk, skb, cur_mss); |
| tcp_adjust_pcount(sk, skb, oldpcount - tcp_skb_pcount(skb)); |
| } |