From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Abeni Subject: Re: [PATCH net] udp: properly cope with csum errors Date: Mon, 06 Feb 2017 10:08:02 +0100 Message-ID: <1486372082.2514.3.camel@redhat.com> References: <1486315524.7793.22.camel@edumazet-glaptop3.roam.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Cc: netdev , Hannes Frederic Sowa , Dmitry Vyukov To: Eric Dumazet , David Miller Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51820 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751002AbdBFJID (ORCPT ); Mon, 6 Feb 2017 04:08:03 -0500 In-Reply-To: <1486315524.7793.22.camel@edumazet-glaptop3.roam.corp.google.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sun, 2017-02-05 at 09:25 -0800, Eric Dumazet wrote: > From: Eric Dumazet > > Dmitry reported that UDP sockets being destroyed would trigger the > WARN_ON(atomic_read(&sk->sk_rmem_alloc)); in inet_sock_destruct() > > It turns out we do not properly destroy skb(s) that have wrong UDP > checksum. > > Thanks again to syzkaller team. > > Fixes : 7c13f97ffde6 ("udp: do fwd memory scheduling on dequeue") > Reported-by: Dmitry Vyukov > Signed-off-by: Eric Dumazet > Cc: Paolo Abeni > Cc: Hannes Frederic Sowa > --- >  include/net/sock.h  |    4 +++- >  net/core/datagram.c |    8 ++++++-- >  net/ipv4/udp.c      |    2 +- >  net/ipv6/udp.c      |    2 +- >  4 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index f0e867f58722..c4f5e6fca17c 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -2006,7 +2006,9 @@ void sk_reset_timer(struct sock *sk, struct timer_list *timer, >  void sk_stop_timer(struct sock *sk, struct timer_list *timer); >   >  int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb, > - unsigned int flags); > + unsigned int flags, > + void (*destructor)(struct sock *sk, > +    struct sk_buff *skb)); >  int __sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb); >  int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb); >   > diff --git a/net/core/datagram.c b/net/core/datagram.c > index 662bea587165..ea633342ab0d 100644 > --- a/net/core/datagram.c > +++ b/net/core/datagram.c > @@ -332,7 +332,9 @@ void __skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb, int len) >  EXPORT_SYMBOL(__skb_free_datagram_locked); >   >  int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb, > - unsigned int flags) > + unsigned int flags, > + void (*destructor)(struct sock *sk, > +    struct sk_buff *skb)) >  { >   int err = 0; >   > @@ -342,6 +344,8 @@ int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb, >   if (skb == skb_peek(&sk->sk_receive_queue)) { >   __skb_unlink(skb, &sk->sk_receive_queue); >   atomic_dec(&skb->users); > + if (destructor) > + destructor(sk, skb); >   err = 0; >   } >   spin_unlock_bh(&sk->sk_receive_queue.lock); > @@ -375,7 +379,7 @@ EXPORT_SYMBOL(__sk_queue_drop_skb); >   >  int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags) >  { > - int err = __sk_queue_drop_skb(sk, skb, flags); > + int err = __sk_queue_drop_skb(sk, skb, flags, NULL); >   >   kfree_skb(skb); >   sk_mem_reclaim_partial(sk); > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index 1307a7c2e544..8aab7d78d25b 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -1501,7 +1501,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, >   return err; >   >  csum_copy_err: > - if (!__sk_queue_drop_skb(sk, skb, flags)) { > + if (!__sk_queue_drop_skb(sk, skb, flags, udp_skb_destructor)) { >   UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite); >   UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); >   } > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > index 4d5c4eee4b3f..8990856f5101 100644 > --- a/net/ipv6/udp.c > +++ b/net/ipv6/udp.c > @@ -441,7 +441,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, >   return err; >   >  csum_copy_err: > - if (!__sk_queue_drop_skb(sk, skb, flags)) { > + if (!__sk_queue_drop_skb(sk, skb, flags, udp_skb_destructor)) { >   if (is_udp4) { >   UDP_INC_STATS(sock_net(sk), >         UDP_MIB_CSUMERRORS, is_udplite); LGTM, Thanks Eric for fixing this! Acked-by: Paolo Abeni