Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
MIME-Version: 1.0
References: <3793723.1694795079@warthog.procyon.org.uk> <CANn89iLwMhOnrmQTZJ+BqZJSbJZ+Q4W6xRknAAr+uSrk5TX-EQ@mail.gmail.com>
 <0000000000001c12b30605378ce8@google.com> <3905046.1695031382@warthog.procyon.org.uk>
 <65085768c17da_898cd294ae@willemb.c.googlers.com.notmuch>
In-Reply-To: <65085768c17da_898cd294ae@willemb.c.googlers.com.notmuch>
From:   Eric Dumazet <edumazet@google.com>
Date:   Mon, 18 Sep 2023 16:04:49 +0200
Message-ID: <CANn89iJ39Hguu6bRm2am6J_u0pSnm++ORa_UVpC0+8-mxORFfw@mail.gmail.com>
Subject: Re: [syzbot] [net?] WARNING in __ip6_append_data
To:     Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc:     David Howells <dhowells@redhat.com>,
        syzbot <syzbot+62cbf263225ae13ff153@syzkaller.appspotmail.com>,
        bpf@vger.kernel.org, davem@davemloft.net, dsahern@kernel.org,
        kuba@kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, pabeni@redhat.com,
        syzkaller-bugs@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Sep 18, 2023 at 3:58=E2=80=AFPM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> David Howells wrote:
> > David Howells <dhowells@redhat.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 w=
ith
> > > EINVAL before it gets into __ip6_append_data().
> >
> > Actually, it only fails with EINVAL if the size is not a multiple of th=
e block
> > size of the source file because it's open O_DIRECT so, say, 65536-512 i=
s fine
> > (and works).
> >
> > But thinking more on this further, is this even a bug in my code, I won=
der?
> > 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 inde=
ed
> what the repro does.

l2tp_ip6_sendmsg() claims ip6_append_data() can make better checks,
but what about simply replacing INT_MAX by 65535 ?

diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 44cfb72bbd18a34e83e50bebca09729c55df524f..ab57a134923bfc8040dba0d8fb7=
02551ff265184
100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -502,10 +502,7 @@ static int l2tp_ip6_sendmsg(struct sock *sk,
struct msghdr *msg, size_t len)
        int ulen;
        int err;

-       /* Rough check on arithmetic overflow,
-        * better check is made in ip6_append_data().
-        */
-       if (len > INT_MAX - transhdrlen)
+       if (len > 65535 - transhdrlen)
                return -EMSGSIZE;
        ulen =3D len + transhdrlen;