netdev - Re: [Intel-wired-lan] bug with rx-udp-gro-forwarding offloading?

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: <CANn89i+F=R71refT8K_8hPaP+uWn15GeHz+FTMYU=VPTG24WFA@mail.gmail.com>
Date: Thu, 6 Jul 2023 15:56:38 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>, Willem de Bruijn <willemb@...gle.com>
Cc: Ian Kumlien <ian.kumlien@...il.com>, Alexander Lobakin <aleksander.lobakin@...el.com>, 
	intel-wired-lan <intel-wired-lan@...ts.osuosl.org>, Jakub Kicinski <kuba@...nel.org>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Intel-wired-lan] bug with rx-udp-gro-forwarding offloading?

On Thu, Jul 6, 2023 at 3:02 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> On Thu, 2023-07-06 at 13:27 +0200, Ian Kumlien wrote:
> > On Thu, Jul 6, 2023 at 10:42 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > > On Wed, 2023-07-05 at 15:58 +0200, Ian Kumlien wrote:
> > > > On Wed, Jul 5, 2023 at 3:29 PM Paolo Abeni <pabeni@...hat.com> wrote:
> > > > >
> > > > > On Wed, 2023-07-05 at 13:32 +0200, Ian Kumlien wrote:
> > > > > > On Wed, Jul 5, 2023 at 12:28 PM Paolo Abeni <pabeni@...hat.com> wrote:
> > > > > > >
> > > > > > > On Tue, 2023-07-04 at 16:27 +0200, Ian Kumlien wrote:
> > > > > > > > More stacktraces.. =)
> > > > > > > >
> > > > > > > > cat bug.txt | ./scripts/decode_stacktrace.sh vmlinux
> > > > > > > > [  411.413767] ------------[ cut here ]------------
> > > > > > > > [  411.413792] WARNING: CPU: 9 PID: 942 at include/net/ud     p.h:509
> > > > > > > > udpv6_queue_rcv_skb (./include/net/udp.h:509 net/ipv6/udp.c:800
> > > > > > > > net/ipv6/udp.c:787)
> > > > > > >
> > > > > > > I'm really running out of ideas here...
> > > > > > >
> > > > > > > This is:
> > > > > > >
> > > > > > >         WARN_ON_ONCE(UDP_SKB_CB(skb)->partial_cov);
> > > > > > >
> > > > > > > sort of hint skb being shared (skb->users > 1) while enqueued in
> > > > > > > multiple places (bridge local input and br forward/flood to tun
> > > > > > > device). I audited the bridge mc flooding code, and I could not find
> > > > > > > how a shared skb could land into the local input path.
> > > > > > >
> > > > > > > Anyway the other splats reported here and in later emails are
> > > > > > > compatible with shared skbs.
> > > > > > >
> > > > > > > The above leads to another bunch of questions:
> > > > > > > * can you reproduce the issue after disabling 'rx-gro-list' on the
> > > > > > > ingress device? (while keeping 'rx-udp-gro-forwarding' on).
> > > > > >
> > > > > > With rx-gro-list off, as in never turned on, everything seems to run fine
> > > > > >
> > > > > > > * do you have by chance qdiscs on top of the VM tun devices?
> > > > > >
> > > > > > default qdisc is fq
> > > > >
> > > > > IIRC libvirt could reset the qdisc to noqueue for the owned tun
> > > > > devices.
> > > > >
> > > > > Could you please report the output of:
> > > > >
> > > > > tc -d -s qdisc show dev <tun dev name>
> > > >
> > > > I don't have these set:
> > > > CONFIG_NET_SCH_INGRESS
> > > > CONFIG_NET_SCHED
> > > >
> > > > so tc just gives an error...
> > >
> > > The above is confusing. AS CONFIG_NET_SCH_DEFAULT depends on
> > > CONFIG_NET_SCHED, you should not have a default qdisc, too ;)
> >
> > Well it's still set in sysctl - dunno if it fails
> >
> > > Could you please share your kernel config?
> >
> > Sure...
> >
> > As a side note, it hasn't crashed - no traces since we did the last change
>
> It sounds like an encouraging sing! (last famous words...). I'll wait 1
> more day, than I'll submit formally...
>
> > For reference, this is git diff on the running kernels source tree:
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index cea28d30abb5..1b2394ebaf33 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -4270,6 +4270,17 @@ struct sk_buff *skb_segment_list(struct sk_buff *skb,
> >
> >         skb_push(skb, -skb_network_offset(skb) + offset);
> >
> > +       if (WARN_ON_ONCE(skb_shared(skb))) {
> > +               skb = skb_share_check(skb, GFP_ATOMIC);
> > +               if (!skb)
> > +                       goto err_linearize;
> > +       }
> > +
> > +       /* later code will clear the gso area in the shared info */
> > +       err = skb_header_unclone(skb, GFP_ATOMIC);
> > +       if (err)
> > +               goto err_linearize;
> > +
> >         skb_shinfo(skb)->frag_list = NULL;
> >
> >         while (list_skb) {
>
> ...the above check only, as the other 2 should only catch-up side
> effects of lack of this one. In any case the above address a real
> issue, so we likely want it no-matter-what.
>

Interesting, I wonder if this could also fix some syzbot reports
Willem and I are investigating.

Any idea of when the bug was 'added' or 'revealed' ?