Re: [PATCH net-next] udp: avoid refcount_t saturation in __udp_gso_segment()

2018-05-11 Thread David Miller
From: Eric Dumazet 
Date: Thu, 10 May 2018 19:07:13 -0700

> For some reason, Willem thought that the issue we fixed for TCP
> in commit 7ec318feeed1 ("tcp: gso: avoid refcount_t warning from
> tcp_gso_segment()") was not relevant for UDP GSO.
> 
> But syzbot found its way.
 ...
> Fixes: ad405857b174 ("udp: better wmem accounting on gso")
> Signed-off-by: Eric Dumazet 
> Cc: Willem de Bruijn 
> Cc: Alexander Duyck 
> Reported-by: syzbot 

Applied, thanks Eric.


Re: [PATCH net-next] udp: avoid refcount_t saturation in __udp_gso_segment()

2018-05-10 Thread Willem de Bruijn
On Thu, May 10, 2018 at 10:07 PM, Eric Dumazet  wrote:
> For some reason, Willem thought that the issue we fixed for TCP
> in commit 7ec318feeed1 ("tcp: gso: avoid refcount_t warning from
> tcp_gso_segment()") was not relevant for UDP GSO.
>
> But syzbot found its way.

[..]

> Fixes: ad405857b174 ("udp: better wmem accounting on gso")
> Signed-off-by: Eric Dumazet 
> Cc: Willem de Bruijn 
> Cc: Alexander Duyck 
> Reported-by: syzbot 

Acked-by: Willem de Bruijn 

Thanks Eric. Yep, I was naive here. I am quite curious what kind of
gso packet syzkaller was able to cook that exceeds the truesize
of its segments.


[PATCH net-next] udp: avoid refcount_t saturation in __udp_gso_segment()

2018-05-10 Thread Eric Dumazet
For some reason, Willem thought that the issue we fixed for TCP
in commit 7ec318feeed1 ("tcp: gso: avoid refcount_t warning from
tcp_gso_segment()") was not relevant for UDP GSO.

But syzbot found its way.

refcount_t: saturated; leaking memory.
WARNING: CPU: 0 PID: 10261 at lib/refcount.c:78 
refcount_add_not_zero+0x2d4/0x320 lib/refcount.c:78
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 10261 Comm: syz-executor5 Not tainted 4.17.0-rc3+ #38
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1b9/0x294 lib/dump_stack.c:113
 panic+0x22f/0x4de kernel/panic.c:184
 __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
 report_bug+0x252/0x2d0 lib/bug.c:186
 fixup_bug arch/x86/kernel/traps.c:178 [inline]
 do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
 do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:refcount_add_not_zero+0x2d4/0x320 lib/refcount.c:78
RSP: 0018:880196db6b90 EFLAGS: 00010282
RAX: 0026 RBX: ff01 RCX: c900040d9000
RDX: 4a29 RSI: 8160f6f1 RDI: 880196db66f0
RBP: 880196db6c78 R08: 8801b33d6740 R09: 0002
R10: 8801b33d6740 R11:  R12: 
R13:  R14: 880196db6c50 R15: 00020101
 refcount_add+0x1b/0x70 lib/refcount.c:102
 __udp_gso_segment+0xaa5/0xee0 net/ipv4/udp_offload.c:272
 udp4_ufo_fragment+0x592/0x7a0 net/ipv4/udp_offload.c:301
 inet_gso_segment+0x639/0x12b0 net/ipv4/af_inet.c:1342
 skb_mac_gso_segment+0x3ad/0x720 net/core/dev.c:2792
 __skb_gso_segment+0x3bb/0x870 net/core/dev.c:2865
 skb_gso_segment include/linux/netdevice.h:4050 [inline]
 validate_xmit_skb+0x54d/0xd90 net/core/dev.c:3122
 __dev_queue_xmit+0xbf8/0x34c0 net/core/dev.c:3579
 dev_queue_xmit+0x17/0x20 net/core/dev.c:3620
 neigh_direct_output+0x15/0x20 net/core/neighbour.c:1401
 neigh_output include/net/neighbour.h:483 [inline]
 ip_finish_output2+0xa5f/0x1840 net/ipv4/ip_output.c:229
 ip_finish_output+0x828/0xf80 net/ipv4/ip_output.c:317
 NF_HOOK_COND include/linux/netfilter.h:277 [inline]
 ip_output+0x21b/0x850 net/ipv4/ip_output.c:405
 dst_output include/net/dst.h:444 [inline]
 ip_local_out+0xc5/0x1b0 net/ipv4/ip_output.c:124
 ip_send_skb+0x40/0xe0 net/ipv4/ip_output.c:1434
 udp_send_skb.isra.37+0x5eb/0x1000 net/ipv4/udp.c:825
 udp_push_pending_frames+0x5c/0xf0 net/ipv4/udp.c:853
 udp_v6_push_pending_frames+0x380/0x3e0 net/ipv6/udp.c:1105
 udp_lib_setsockopt+0x59a/0x600 net/ipv4/udp.c:2403
 udpv6_setsockopt+0x95/0xa0 net/ipv6/udp.c:1447
 sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:3046
 __sys_setsockopt+0x1bd/0x390 net/socket.c:1903
 __do_sys_setsockopt net/socket.c:1914 [inline]
 __se_sys_setsockopt net/socket.c:1911 [inline]
 __x64_sys_setsockopt+0xbe/0x150 net/socket.c:1911
 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Fixes: ad405857b174 ("udp: better wmem accounting on gso")
Signed-off-by: Eric Dumazet 
Cc: Willem de Bruijn 
Cc: Alexander Duyck 
Reported-by: syzbot 
---
 net/ipv4/udp_offload.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 
ede2a7305b90f789c748d911530453ec2cbbfab7..92dc9e5a7ff3d0a7509bfa2a66e9189c8341a5fa
 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -268,9 +268,17 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
uh->check = gso_make_checksum(seg, ~check) ? : CSUM_MANGLED_0;
 
/* update refcount for the packet */
-   if (copy_dtor)
-   refcount_add(sum_truesize - gso_skb->truesize,
-&sk->sk_wmem_alloc);
+   if (copy_dtor) {
+   int delta = sum_truesize - gso_skb->truesize;
+
+   /* In some pathological cases, delta can be negative.
+* We need to either use refcount_add() or 
refcount_sub_and_test()
+*/
+   if (likely(delta >= 0))
+   refcount_add(delta, &sk->sk_wmem_alloc);
+   else
+   WARN_ON_ONCE(refcount_sub_and_test(-delta, 
&sk->sk_wmem_alloc));
+   }
return segs;
 }
 EXPORT_SYMBOL_GPL(__udp_gso_segment);
-- 
2.17.0.441.gb46fe60e1d-goog