netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* net/ipv4: division by 0 in tcp_select_window
@ 2017-03-03 18:10 Dmitry Vyukov
  2017-03-03 18:24 ` Dmitry Vyukov
  2017-03-03 18:25 ` Eric Dumazet
  0 siblings, 2 replies; 9+ messages in thread
From: Dmitry Vyukov @ 2017-03-03 18:10 UTC (permalink / raw)
  To: David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, LKML, Eric Dumazet, Cong Wang
  Cc: syzkaller

Hello,

The following program triggers division by 0 in tcp_select_window:

https://gist.githubusercontent.com/dvyukov/ef28c0fd2ab57a655508ef7621b12e6c/raw/079011e2a9523a390b0621cbc1e5d9d5e637fd6d/gistfile1.txt

divide error: 0000 [#1] SMP KASAN
Modules linked in:
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.10.0+ #270
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88006c236340 task.stack: ffff88006c248000
RIP: 0010:__tcp_select_window+0x6db/0x920 net/ipv4/tcp_output.c:2585
RSP: 0018:ffff88006cf86b40 EFLAGS: 00010206
RAX: 00000000000000c4 RBX: ffff88006cf86cd8 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8800686228bd
RBP: ffff88006cf86d00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000004 R11: ffffed000d9f0e18 R12: 00000000000000c4
R13: 000000000000a700 R14: ffff880068622040 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff88006cf80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020e5cff8 CR3: 0000000005021000 CR4: 00000000001406e0
Call Trace:
 <IRQ>
 tcp_select_window net/ipv4/tcp_output.c:270 [inline]
 tcp_transmit_skb+0xc35/0x3460 net/ipv4/tcp_output.c:1014
 tcp_xmit_probe_skb+0x36d/0x440 net/ipv4/tcp_output.c:3528
 tcp_write_wakeup+0x23b/0x6d0 net/ipv4/tcp_output.c:3577
 tcp_send_probe0+0xbf/0x5d0 net/ipv4/tcp_output.c:3593
 tcp_probe_timer net/ipv4/tcp_timer.c:362 [inline]
 tcp_write_timer_handler+0x849/0x9d0 net/ipv4/tcp_timer.c:578
 tcp_write_timer+0x164/0x180 net/ipv4/tcp_timer.c:592
 call_timer_fn+0x241/0x820 kernel/time/timer.c:1266
 expire_timers kernel/time/timer.c:1305 [inline]
 __run_timers+0x960/0xcf0 kernel/time/timer.c:1599
 run_timer_softirq+0x21/0x80 kernel/time/timer.c:1612
 __do_softirq+0x31f/0xbe7 kernel/softirq.c:284
 invoke_softirq kernel/softirq.c:364 [inline]
 irq_exit+0x1cc/0x200 kernel/softirq.c:405
 exiting_irq arch/x86/include/asm/apic.h:658 [inline]
 smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:962
 apic_timer_interrupt+0x93/0xa0 arch/x86/entry/entry_64.S:487
RIP: 0010:native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:53
RSP: 0018:ffff88006c24fc10 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff10
RAX: dffffc0000000000 RBX: 1ffff1000d849f85 RCX: 0000000000000000
RDX: 1ffffffff0a18ebc RSI: 0000000000000001 RDI: ffffffff850c75e0
RBP: ffff88006c24fc10 R08: ffff88007fff70dc R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 1ffff1000d849fa9
R13: ffff88006c24fcc8 R14: ffffffff856972b8 R15: ffff88006c24fe68
 </IRQ>
 arch_safe_halt arch/x86/include/asm/paravirt.h:98 [inline]
 default_idle+0xbf/0x440 arch/x86/kernel/process.c:271
 arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:262
 default_idle_call+0x36/0x90 kernel/sched/idle.c:96
 cpuidle_idle_call kernel/sched/idle.c:154 [inline]
 do_idle+0x373/0x520 kernel/sched/idle.c:243
 cpu_startup_entry+0x18/0x20 kernel/sched/idle.c:345
 start_secondary+0x36c/0x460 arch/x86/kernel/smpboot.c:272
 start_cpu+0x14/0x14 arch/x86/kernel/head_64.S:306
Code: 5d c3 e8 99 0d e5 fd 45 85 ff 44 89 bd 74 fe ff ff 0f 8f 14 fc
ff ff 45 31 e4 eb 93 e8 7f 0d e5 fd 8b b5 74 fe ff ff 44 89 e0 99 <f7>
fe 41 89 c4 44 0f af e6 e9 71 ff ff ff e8 62 0d e5 fd 4c 89
RIP: __tcp_select_window+0x6db/0x920 net/ipv4/tcp_output.c:2585 RSP:
ffff88006cf86b40
---[ end trace 5efcbe8231e36800 ]---

On commit c82be9d2244aacea9851c86f4fb74694c99cd874.

The guy that resets mss seems to be
inet_csk_listen_start->inet_csk_delack_init. After that the timer
fires and divides by icsk->icsk_ack.rcv_mss==0.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: net/ipv4: division by 0 in tcp_select_window
  2017-03-03 18:10 net/ipv4: division by 0 in tcp_select_window Dmitry Vyukov
@ 2017-03-03 18:24 ` Dmitry Vyukov
  2017-03-03 18:37   ` Eric Dumazet
  2017-03-03 18:25 ` Eric Dumazet
  1 sibling, 1 reply; 9+ messages in thread
From: Dmitry Vyukov @ 2017-03-03 18:24 UTC (permalink / raw)
  To: David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, LKML, Eric Dumazet, Cong Wang
  Cc: syzkaller

On Fri, Mar 3, 2017 at 7:10 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> Hello,
>
> The following program triggers division by 0 in tcp_select_window:
>
> https://gist.githubusercontent.com/dvyukov/ef28c0fd2ab57a655508ef7621b12e6c/raw/079011e2a9523a390b0621cbc1e5d9d5e637fd6d/gistfile1.txt
>
> divide error: 0000 [#1] SMP KASAN
> Modules linked in:
> CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.10.0+ #270
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: ffff88006c236340 task.stack: ffff88006c248000
> RIP: 0010:__tcp_select_window+0x6db/0x920 net/ipv4/tcp_output.c:2585
> RSP: 0018:ffff88006cf86b40 EFLAGS: 00010206
> RAX: 00000000000000c4 RBX: ffff88006cf86cd8 RCX: dffffc0000000000
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8800686228bd
> RBP: ffff88006cf86d00 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000004 R11: ffffed000d9f0e18 R12: 00000000000000c4
> R13: 000000000000a700 R14: ffff880068622040 R15: 0000000000000000
> FS:  0000000000000000(0000) GS:ffff88006cf80000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020e5cff8 CR3: 0000000005021000 CR4: 00000000001406e0
> Call Trace:
>  <IRQ>
>  tcp_select_window net/ipv4/tcp_output.c:270 [inline]
>  tcp_transmit_skb+0xc35/0x3460 net/ipv4/tcp_output.c:1014
>  tcp_xmit_probe_skb+0x36d/0x440 net/ipv4/tcp_output.c:3528
>  tcp_write_wakeup+0x23b/0x6d0 net/ipv4/tcp_output.c:3577
>  tcp_send_probe0+0xbf/0x5d0 net/ipv4/tcp_output.c:3593
>  tcp_probe_timer net/ipv4/tcp_timer.c:362 [inline]
>  tcp_write_timer_handler+0x849/0x9d0 net/ipv4/tcp_timer.c:578
>  tcp_write_timer+0x164/0x180 net/ipv4/tcp_timer.c:592
>  call_timer_fn+0x241/0x820 kernel/time/timer.c:1266
>  expire_timers kernel/time/timer.c:1305 [inline]
>  __run_timers+0x960/0xcf0 kernel/time/timer.c:1599
>  run_timer_softirq+0x21/0x80 kernel/time/timer.c:1612
>  __do_softirq+0x31f/0xbe7 kernel/softirq.c:284
>  invoke_softirq kernel/softirq.c:364 [inline]
>  irq_exit+0x1cc/0x200 kernel/softirq.c:405
>  exiting_irq arch/x86/include/asm/apic.h:658 [inline]
>  smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:962
>  apic_timer_interrupt+0x93/0xa0 arch/x86/entry/entry_64.S:487
> RIP: 0010:native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:53
> RSP: 0018:ffff88006c24fc10 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff10
> RAX: dffffc0000000000 RBX: 1ffff1000d849f85 RCX: 0000000000000000
> RDX: 1ffffffff0a18ebc RSI: 0000000000000001 RDI: ffffffff850c75e0
> RBP: ffff88006c24fc10 R08: ffff88007fff70dc R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 1ffff1000d849fa9
> R13: ffff88006c24fcc8 R14: ffffffff856972b8 R15: ffff88006c24fe68
>  </IRQ>
>  arch_safe_halt arch/x86/include/asm/paravirt.h:98 [inline]
>  default_idle+0xbf/0x440 arch/x86/kernel/process.c:271
>  arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:262
>  default_idle_call+0x36/0x90 kernel/sched/idle.c:96
>  cpuidle_idle_call kernel/sched/idle.c:154 [inline]
>  do_idle+0x373/0x520 kernel/sched/idle.c:243
>  cpu_startup_entry+0x18/0x20 kernel/sched/idle.c:345
>  start_secondary+0x36c/0x460 arch/x86/kernel/smpboot.c:272
>  start_cpu+0x14/0x14 arch/x86/kernel/head_64.S:306
> Code: 5d c3 e8 99 0d e5 fd 45 85 ff 44 89 bd 74 fe ff ff 0f 8f 14 fc
> ff ff 45 31 e4 eb 93 e8 7f 0d e5 fd 8b b5 74 fe ff ff 44 89 e0 99 <f7>
> fe 41 89 c4 44 0f af e6 e9 71 ff ff ff e8 62 0d e5 fd 4c 89
> RIP: __tcp_select_window+0x6db/0x920 net/ipv4/tcp_output.c:2585 RSP:
> ffff88006cf86b40
> ---[ end trace 5efcbe8231e36800 ]---
>
> On commit c82be9d2244aacea9851c86f4fb74694c99cd874.
>
> The guy that resets mss seems to be
> inet_csk_listen_start->inet_csk_delack_init. After that the timer
> fires and divides by icsk->icsk_ack.rcv_mss==0.


Wonder if this has been causing other crashes like this one?

------------[ cut here ]------------
kernel BUG at net/ipv4/tcp_output.c:2748!
Call Trace:
 <IRQ>
 tcp_retransmit_skb+0x2e/0x230 net/ipv4/tcp_output.c:2822
 tcp_retransmit_timer+0x104c/0x2d50 net/ipv4/tcp_timer.c:491
 tcp_write_timer_handler+0x334/0x9d0 net/ipv4/tcp_timer.c:574
 tcp_write_timer+0x164/0x180 net/ipv4/tcp_timer.c:592
 call_timer_fn+0x241/0x820 kernel/time/timer.c:1266
 expire_timers kernel/time/timer.c:1305 [inline]
 __run_timers+0x960/0xcf0 kernel/time/timer.c:1599
 run_timer_softirq+0x21/0x80 kernel/time/timer.c:1612
 __do_softirq+0x31f/0xbe7 kernel/softirq.c:284
 invoke_softirq kernel/softirq.c:364 [inline]
 irq_exit+0x1cc/0x200 kernel/softirq.c:405
 exiting_irq arch/x86/include/asm/apic.h:658 [inline]
 smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:962
 apic_timer_interrupt+0x93/0xa0 arch/x86/entry/entry_64.S:487

if (before(TCP_SKB_CB(skb)->seq, tp->snd_una)) {
  if (before(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
    BUG();

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: net/ipv4: division by 0 in tcp_select_window
  2017-03-03 18:10 net/ipv4: division by 0 in tcp_select_window Dmitry Vyukov
  2017-03-03 18:24 ` Dmitry Vyukov
@ 2017-03-03 18:25 ` Eric Dumazet
  2017-03-03 20:56   ` Eric Dumazet
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2017-03-03 18:25 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, LKML, Cong Wang, syzkaller

On Fri, Mar 3, 2017 at 10:10 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> Hello,
>
> The following program triggers division by 0 in tcp_select_window:
>
> https://gist.githubusercontent.com/dvyukov/ef28c0fd2ab57a655508ef7621b12e6c/raw/079011e2a9523a390b0621cbc1e5d9d5e637fd6d/gistfile1.txt

Yeah, tcp_disconnect() should never have existed in the first place.

We'll send a patch, unless you take care of this before us .

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: net/ipv4: division by 0 in tcp_select_window
  2017-03-03 18:24 ` Dmitry Vyukov
@ 2017-03-03 18:37   ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2017-03-03 18:37 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, LKML, Cong Wang, syzkaller

On Fri, Mar 3, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Mar 3, 2017 at 7:10 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> Hello,
>>

> Wonder if this has been causing other crashes like this one?
>
> ------------[ cut here ]------------
> kernel BUG at net/ipv4/tcp_output.c:2748!
> Call Trace:
>  <IRQ>
>  tcp_retransmit_skb+0x2e/0x230 net/ipv4/tcp_output.c:2822
>  tcp_retransmit_timer+0x104c/0x2d50 net/ipv4/tcp_timer.c:491
>  tcp_write_timer_handler+0x334/0x9d0 net/ipv4/tcp_timer.c:574
>  tcp_write_timer+0x164/0x180 net/ipv4/tcp_timer.c:592
>  call_timer_fn+0x241/0x820 kernel/time/timer.c:1266
>  expire_timers kernel/time/timer.c:1305 [inline]
>  __run_timers+0x960/0xcf0 kernel/time/timer.c:1599
>  run_timer_softirq+0x21/0x80 kernel/time/timer.c:1612
>  __do_softirq+0x31f/0xbe7 kernel/softirq.c:284
>  invoke_softirq kernel/softirq.c:364 [inline]
>  irq_exit+0x1cc/0x200 kernel/softirq.c:405
>  exiting_irq arch/x86/include/asm/apic.h:658 [inline]
>  smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:962
>  apic_timer_interrupt+0x93/0xa0 arch/x86/entry/entry_64.S:487
>
> if (before(TCP_SKB_CB(skb)->seq, tp->snd_una)) {
>   if (before(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
>     BUG();

This path uses a socket lock. Probably different problem.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: net/ipv4: division by 0 in tcp_select_window
  2017-03-03 18:25 ` Eric Dumazet
@ 2017-03-03 20:56   ` Eric Dumazet
  2017-03-03 22:08     ` [PATCH net] tcp: fix various issues for sockets morphing to listen state Eric Dumazet
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2017-03-03 20:56 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Dmitry Vyukov, David Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Cong Wang,
	syzkaller

On Fri, 2017-03-03 at 10:25 -0800, Eric Dumazet wrote:
> On Fri, Mar 3, 2017 at 10:10 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > Hello,
> >
> > The following program triggers division by 0 in tcp_select_window:
> >
> > https://gist.githubusercontent.com/dvyukov/ef28c0fd2ab57a655508ef7621b12e6c/raw/079011e2a9523a390b0621cbc1e5d9d5e637fd6d/gistfile1.txt
> 
> Yeah, tcp_disconnect() should never have existed in the first place.
> 
> We'll send a patch, unless you take care of this before us .

Could you try this first patch ?

Probably others will also be needed.

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 40d893556e6701ace6a02903e53c45822d6fa56d..2187ebf1f270d19e6dd019b8f9df5eef8d018e03 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -552,7 +552,8 @@ void tcp_write_timer_handler(struct sock *sk)
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	int event;
 
-	if (sk->sk_state == TCP_CLOSE || !icsk->icsk_pending)
+	if (((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
+	    !icsk->icsk_pending)
 		goto out;
 
 	if (time_after(icsk->icsk_timeout, jiffies)) {

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net] tcp: fix various issues for sockets morphing to listen state
  2017-03-03 20:56   ` Eric Dumazet
@ 2017-03-03 22:08     ` Eric Dumazet
  2017-03-05 12:00       ` Dmitry Vyukov
  2017-03-07 21:59       ` David Miller
  0 siblings, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2017-03-03 22:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Dmitry Vyukov, David Miller, netdev

From: Eric Dumazet <edumazet@google.com>

Dmitry Vyukov reported a divide by 0 triggered by syzkaller, exploiting
tcp_disconnect() path that was never really considered and/or used
before syzkaller ;)

I was not able to reproduce the bug, but it seems issues here are the
three possible actions that assumed they would never trigger on a
listener.

1) tcp_write_timer_handler
2) tcp_delack_timer_handler
3) MTU reduction

Only IPv6 MTU reduction was properly testing TCP_CLOSE and TCP_LISTEN
 states from tcp_v6_mtu_reduced()


Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
---
 net/ipv4/tcp_ipv4.c  |    7 +++++--
 net/ipv4/tcp_timer.c |    6 ++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9a89b8deafae1e9b2e8d1d9bc211c9c30b8dd8ec..8f3ec1365497a58972d31d419f88b34457b5ae39 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -279,10 +279,13 @@ EXPORT_SYMBOL(tcp_v4_connect);
  */
 void tcp_v4_mtu_reduced(struct sock *sk)
 {
-	struct dst_entry *dst;
 	struct inet_sock *inet = inet_sk(sk);
-	u32 mtu = tcp_sk(sk)->mtu_info;
+	struct dst_entry *dst;
+	u32 mtu;
 
+	if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))
+		return;
+	mtu = tcp_sk(sk)->mtu_info;
 	dst = inet_csk_update_pmtu(sk, mtu);
 	if (!dst)
 		return;
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 40d893556e6701ace6a02903e53c45822d6fa56d..b2ab411c6d3728fa7dbdebde045532a7317f5166 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -249,7 +249,8 @@ void tcp_delack_timer_handler(struct sock *sk)
 
 	sk_mem_reclaim_partial(sk);
 
-	if (sk->sk_state == TCP_CLOSE || !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
+	if (((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
+	    !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
 		goto out;
 
 	if (time_after(icsk->icsk_ack.timeout, jiffies)) {
@@ -552,7 +553,8 @@ void tcp_write_timer_handler(struct sock *sk)
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	int event;
 
-	if (sk->sk_state == TCP_CLOSE || !icsk->icsk_pending)
+	if (((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
+	    !icsk->icsk_pending)
 		goto out;
 
 	if (time_after(icsk->icsk_timeout, jiffies)) {

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: fix various issues for sockets morphing to listen state
  2017-03-03 22:08     ` [PATCH net] tcp: fix various issues for sockets morphing to listen state Eric Dumazet
@ 2017-03-05 12:00       ` Dmitry Vyukov
  2017-03-05 16:58         ` Eric Dumazet
  2017-03-07 21:59       ` David Miller
  1 sibling, 1 reply; 9+ messages in thread
From: Dmitry Vyukov @ 2017-03-05 12:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Eric Dumazet, David Miller, netdev

On Fri, Mar 3, 2017 at 11:08 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Dmitry Vyukov reported a divide by 0 triggered by syzkaller, exploiting
> tcp_disconnect() path that was never really considered and/or used
> before syzkaller ;)
>
> I was not able to reproduce the bug, but it seems issues here are the
> three possible actions that assumed they would never trigger on a
> listener.
>
> 1) tcp_write_timer_handler
> 2) tcp_delack_timer_handler
> 3) MTU reduction
>
> Only IPv6 MTU reduction was properly testing TCP_CLOSE and TCP_LISTEN
>  states from tcp_v6_mtu_reduced()

I've run the test as "while ./a.out; do true; done". I've also noticed
that it either reproduces very quickly or does not happen at all. So I
rebooted machine, run the program in loop and then repeated if it does
not happen within seconds.

Retested with your patch, seems to not crash anymore.


> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> ---
>  net/ipv4/tcp_ipv4.c  |    7 +++++--
>  net/ipv4/tcp_timer.c |    6 ++++--
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 9a89b8deafae1e9b2e8d1d9bc211c9c30b8dd8ec..8f3ec1365497a58972d31d419f88b34457b5ae39 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -279,10 +279,13 @@ EXPORT_SYMBOL(tcp_v4_connect);
>   */
>  void tcp_v4_mtu_reduced(struct sock *sk)
>  {
> -       struct dst_entry *dst;
>         struct inet_sock *inet = inet_sk(sk);
> -       u32 mtu = tcp_sk(sk)->mtu_info;
> +       struct dst_entry *dst;
> +       u32 mtu;
>
> +       if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))
> +               return;
> +       mtu = tcp_sk(sk)->mtu_info;
>         dst = inet_csk_update_pmtu(sk, mtu);
>         if (!dst)
>                 return;
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 40d893556e6701ace6a02903e53c45822d6fa56d..b2ab411c6d3728fa7dbdebde045532a7317f5166 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -249,7 +249,8 @@ void tcp_delack_timer_handler(struct sock *sk)
>
>         sk_mem_reclaim_partial(sk);
>
> -       if (sk->sk_state == TCP_CLOSE || !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
> +       if (((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
> +           !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
>                 goto out;
>
>         if (time_after(icsk->icsk_ack.timeout, jiffies)) {
> @@ -552,7 +553,8 @@ void tcp_write_timer_handler(struct sock *sk)
>         struct inet_connection_sock *icsk = inet_csk(sk);
>         int event;
>
> -       if (sk->sk_state == TCP_CLOSE || !icsk->icsk_pending)
> +       if (((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
> +           !icsk->icsk_pending)
>                 goto out;
>
>         if (time_after(icsk->icsk_timeout, jiffies)) {
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: fix various issues for sockets morphing to listen state
  2017-03-05 12:00       ` Dmitry Vyukov
@ 2017-03-05 16:58         ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2017-03-05 16:58 UTC (permalink / raw)
  To: Dmitry Vyukov; +Cc: Eric Dumazet, David Miller, netdev

On Sun, Mar 5, 2017 at 4:00 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Mar 3, 2017 at 11:08 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Dmitry Vyukov reported a divide by 0 triggered by syzkaller, exploiting
>> tcp_disconnect() path that was never really considered and/or used
>> before syzkaller ;)
>>
>> I was not able to reproduce the bug, but it seems issues here are the
>> three possible actions that assumed they would never trigger on a
>> listener.
>>
>> 1) tcp_write_timer_handler
>> 2) tcp_delack_timer_handler
>> 3) MTU reduction
>>
>> Only IPv6 MTU reduction was properly testing TCP_CLOSE and TCP_LISTEN
>>  states from tcp_v6_mtu_reduced()
>
> I've run the test as "while ./a.out; do true; done". I've also noticed
> that it either reproduces very quickly or does not happen at all. So I
> rebooted machine, run the program in loop and then repeated if it does
> not happen within seconds.

Yes, I was also doing this.

What I noticed is that your program did not use SO_REUSEADDR, meaning
the second (and later) bind() syscall would fail.

Maybe some sysctl difference between your host and mine....

>
> Retested with your patch, seems to not crash anymore.
>
Thanks for testing !

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: fix various issues for sockets morphing to listen state
  2017-03-03 22:08     ` [PATCH net] tcp: fix various issues for sockets morphing to listen state Eric Dumazet
  2017-03-05 12:00       ` Dmitry Vyukov
@ 2017-03-07 21:59       ` David Miller
  1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2017-03-07 21:59 UTC (permalink / raw)
  To: eric.dumazet; +Cc: edumazet, dvyukov, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 03 Mar 2017 14:08:21 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> Dmitry Vyukov reported a divide by 0 triggered by syzkaller, exploiting
> tcp_disconnect() path that was never really considered and/or used
> before syzkaller ;)
> 
> I was not able to reproduce the bug, but it seems issues here are the
> three possible actions that assumed they would never trigger on a
> listener.
> 
> 1) tcp_write_timer_handler
> 2) tcp_delack_timer_handler
> 3) MTU reduction
> 
> Only IPv6 MTU reduction was properly testing TCP_CLOSE and TCP_LISTEN
>  states from tcp_v6_mtu_reduced()
> 
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-03-08  3:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-03 18:10 net/ipv4: division by 0 in tcp_select_window Dmitry Vyukov
2017-03-03 18:24 ` Dmitry Vyukov
2017-03-03 18:37   ` Eric Dumazet
2017-03-03 18:25 ` Eric Dumazet
2017-03-03 20:56   ` Eric Dumazet
2017-03-03 22:08     ` [PATCH net] tcp: fix various issues for sockets morphing to listen state Eric Dumazet
2017-03-05 12:00       ` Dmitry Vyukov
2017-03-05 16:58         ` Eric Dumazet
2017-03-07 21:59       ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).