netdev - Re: [syzbot] [net?] WARNING in __ip6_append

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] [day] [month] [year] [list]

Message-ID: <CANn89i+fPjwonpWAXOCzrCG+hYH4kwMgJidwZf0CQaoCPOffXA@mail.gmail.com>
Date: Tue, 19 Sep 2023 11:04:41 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Tom Parkin <tparkin@...alix.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, David Howells <dhowells@...hat.com>, 
	syzbot <syzbot+62cbf263225ae13ff153@...kaller.appspotmail.com>, bpf@...r.kernel.org, 
	davem@...emloft.net, dsahern@...nel.org, kuba@...nel.org, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org, pabeni@...hat.com, 
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [net?] WARNING in __ip6_append_data

On Tue, Sep 19, 2023 at 10:27 AM Tom Parkin <tparkin@...alix.com> wrote:
>
> On  Mon, Sep 18, 2023 at 16:04:49 +0200, Eric Dumazet wrote:
> > On Mon, Sep 18, 2023 at 3:58 PM Willem de Bruijn
> > <willemdebruijn.kernel@...il.com> wrote:
> > >
> > > David Howells wrote:
> > > > David Howells <dhowells@...hat.com> wrote:
> > > >
> > > > > I think the attached is probably an equivalent cleaned up reproducer.  Note
> > > > > that if the length given to sendfile() is less than 65536, it fails with
> > > > > EINVAL before it gets into __ip6_append_data().
> > > >
> > > > Actually, it only fails with EINVAL if the size is not a multiple of the block
> > > > size of the source file because it's open O_DIRECT so, say, 65536-512 is fine
> > > > (and works).
> > > >
> > > > But thinking more on this further, is this even a bug in my code, I wonder?
> > > > The length passed is 65536 - but a UDP packet can't carry that, so it
> > > > shouldn't it have errored out before getting that far?  (which is what it
> > > > seems to do when I try it).
> > > >
> > > > I don't see how we get past the length check in ip6_append_data() with the
> > > > reproducer we're given unless the MTU is somewhat bigger than 65536 (is that
> > > > even possible?)
> > >
> > > An ipv6 packet can carry 64KB of payload, so maxnonfragsize of 65535 + 40
> > > sounds correct. But payload length passed of 65536 is not (ignoring ipv6
> > > jumbograms). So that should probably trigger an EINVAL -- if that is indeed
> > > what the repro does.
> >
> > l2tp_ip6_sendmsg() claims ip6_append_data() can make better checks,
> > but what about simply replacing INT_MAX by 65535 ?
>
> Slightly OT but I think the l2tp_ip6.c approach was probably cribbed
> from net/ipv6/udp.c's udpv6_sendmsg originally:
>
>
>     /* Rough check on arithmetic overflow,
>        better check is made in ip6_append_data().
>        */
>     if (len > INT_MAX - sizeof(struct udphdr))
>         return -EMSGSIZE;
>
>
> Should the udp code be modified similarly?
>

Unfortunately both l2tp and udp support CORK (MSG_MORE),
so a modified check like that will not be enough to prevent syzbot reports.

Better than nothing, of course.

I also note that ipv4 size of l2tp does not have any check,
an overflow seems possible with carefully chosen size.