diff mbox series

[net-next] tcp: fix tcp_send_syn_data()

Message ID 1508361630.31614.142.camel@edumazet-glaptop3.roam.corp.google.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series [net-next] tcp: fix tcp_send_syn_data() | expand

Commit Message

Eric Dumazet Oct. 18, 2017, 9:20 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

syn_data was allocated by sk_stream_alloc_skb(), meaning
its destructor and _skb_refdst fields are mangled.

We need to call tcp_skb_tsorted_anchor_cleanup() before
calling kfree_skb() or kernel crashes.

Bug was reported by syzkaller bot.

Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
---
 net/ipv4/tcp_output.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Yuchung Cheng Oct. 18, 2017, 9:40 p.m. UTC | #1 
On Wed, Oct 18, 2017 at 2:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> syn_data was allocated by sk_stream_alloc_skb(), meaning
> its destructor and _skb_refdst fields are mangled.
>
> We need to call tcp_skb_tsorted_anchor_cleanup() before
> calling kfree_skb() or kernel crashes.
>
> Bug was reported by syzkaller bot.
>
> Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>

Thanks for the fix!
> ---
>  net/ipv4/tcp_output.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 53dc1267c85e668d9a6d5d60d24e6101f7a9c56b..988733f289c8c43f3ed88a9ae1b7f272ab8de1a2 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -3383,6 +3383,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
>                 int copied = copy_from_iter(skb_put(syn_data, space), space,
>                                             &fo->data->msg_iter);
>                 if (unlikely(!copied)) {
> +                       tcp_skb_tsorted_anchor_cleanup(syn_data);
>                         kfree_skb(syn_data);
>                         goto fallback;
>                 }
>
>
David Miller Oct. 21, 2017, 12:44 a.m. UTC | #2 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 18 Oct 2017 14:20:30 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> syn_data was allocated by sk_stream_alloc_skb(), meaning
> its destructor and _skb_refdst fields are mangled.
> 
> We need to call tcp_skb_tsorted_anchor_cleanup() before
> calling kfree_skb() or kernel crashes.
> 
> Bug was reported by syzkaller bot.
> 
> Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>

Applied.
diff mbox series

Patch

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 53dc1267c85e668d9a6d5d60d24e6101f7a9c56b..988733f289c8c43f3ed88a9ae1b7f272ab8de1a2 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3383,6 +3383,7 @@  static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
 		int copied = copy_from_iter(skb_put(syn_data, space), space,
 					    &fo->data->msg_iter);
 		if (unlikely(!copied)) {
+			tcp_skb_tsorted_anchor_cleanup(syn_data);
 			kfree_skb(syn_data);
 			goto fallback;
 		}