netdev - RE: UDP data corruption in v4.4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <KL1P15301MB0280926949A1180B671CA516BF720@KL1P15301MB0280.APCP153.PROD.OUTLOOK.COM>
Date:   Mon, 27 Jul 2020 18:57:17 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Eric Dumazet <edumazet@...gle.com>
CC:     Greg KH <greg@...ah.com>, Al Viro <viro@...iv.linux.org.uk>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>, Willy Tarreau <w@....eu>,
        Joseph Salisbury <Joseph.Salisbury@...rosoft.com>,
        Michael Kelley <mikelley@...rosoft.com>
Subject: RE: UDP data corruption in v4.4

> From: Eric Dumazet <edumazet@...gle.com>
> Sent: Monday, July 27, 2020 11:40 AM
> To: Dexuan Cui <decui@...rosoft.com>
> 
> On Mon, Jul 27, 2020 at 11:38 AM Dexuan Cui <decui@...rosoft.com> wrote:
> >
> > > From: Greg KH <greg@...ah.com>
> > > Sent: Friday, July 24, 2020 10:59 PM
> > > > [...]
> > > > Eric Dumazet made an alternative that performs the csum validation
> earlier:
> > > >
> > > > --- a/net/ipv4/udp.c
> > > > +++ b/net/ipv4/udp.c
> > > > @@ -1589,8 +1589,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct
> > > > sk_buff *skb)
> > > >                 }
> > > >         }
> > > >
> > > > -       if (rcu_access_pointer(sk->sk_filter) &&
> > > > -           udp_lib_checksum_complete(skb))
> > > > +       if (udp_lib_checksum_complete(skb))
> > > >                 goto csum_error;
> > > >
> > > >         if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {
> > > >
> > > > I personally like Eric's fix and IMHO we'd better have it in v4.4 rather than
> > > > trying to backport 327868212381.
> > >
> > > Does Eric's fix work with your testing?
> >
> > Yes, it worked in my testing overnight.
> >
> > > If so, great, can you turn it
> > > into something I can apply to the 4.4.y stable tree and send it to
> > > stable@...r.kernel.org?
> > >
> > > greg k-h
> >
> > Will do shortly.
> >
> 
> Just as a reminder, please also add the IPv6 part.
> 
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index
> a8d74f44056a681ef9057c4c4abb34016120b44f..13713e0e5779b75de975faae
> b4511bef40e097dc
> 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -661,8 +661,7 @@ static int udpv6_queue_rcv_one_skb(struct sock
> *sk, struct sk_buff *skb)
>         }
> 
>         prefetch(&sk->sk_rmem_alloc);
> -       if (rcu_access_pointer(sk->sk_filter) &&
> -           udp_lib_checksum_complete(skb))
> +       if (udp_lib_checksum_complete(skb))
>                 goto csum_error;
> 
>         if (sk_filter_trim_cap(sk, skb, sizeof(struct udphdr)))

Oh, yes! :-) Thank you!

Eric, I'll add your Signed-off-by and mine. Please let me know in case
this is not ok.

I'll do a little more testing with the patch and I plan to post the patch
to stable@...r.kernel.org and netdev@...r.kernel.org this afternoon,
i.e. in 3~4 hours or so. 

Thanks,
-- Dexuan