Message ID | 1510096504.2849.100.camel@edumazet-glaptop3.roam.corp.google.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] tcp: gso: avoid refcount_t warning from tcp_gso_segment() | expand |
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 07 Nov 2017 15:15:04 -0800 > From: Eric Dumazet <edumazet@google.com> > > When a GSO skb of truesize O is segmented into 2 new skbs of truesize N1 > and N2, we want to transfer socket ownership to the new fresh skbs. > > In order to avoid expensive atomic operations on a cache line subject to > cache bouncing, we replace the sequence : > > refcount_add(N1, &sk->sk_wmem_alloc); > refcount_add(N2, &sk->sk_wmem_alloc); // repeated by number of segments > > refcount_sub(O, &sk->sk_wmem_alloc); > > by a single > > refcount_add(sum_of(N) - O, &sk->sk_wmem_alloc); > > Problem is : > > In some pathological cases, sum(N) - O might be a negative number, and > syzkaller bot was apparently able to trigger this trace [1] > > > atomic_t was ok with this construct, but we need to take care of the > negative delta with refcount_t ... > Fixes: 14afee4b6092 ("net: convert sock.sk_wmem_alloc from atomic_t to refcount_t") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: syzbot <syzkaller@googlegroups.com> Applied and queued up for -stable, thanks Eric.
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index 11f69bbf93072b7b4dbc3a0485c9f9e0b9ba30b3..443e20a9571ea69339ab49022e0517e939573eb6 100644 --- a/net/ipv4/tcp_offload.c +++ b/net/ipv4/tcp_offload.c @@ -149,11 +149,19 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, * is freed by GSO engine */ if (copy_destructor) { + int delta; + swap(gso_skb->sk, skb->sk); swap(gso_skb->destructor, skb->destructor); sum_truesize += skb->truesize; - refcount_add(sum_truesize - gso_skb->truesize, - &skb->sk->sk_wmem_alloc); + 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, &skb->sk->sk_wmem_alloc); + else + WARN_ON_ONCE(refcount_sub_and_test(-delta, &skb->sk->sk_wmem_alloc)); } delta = htonl(oldlen + (skb_tail_pointer(skb) -