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@xxxxxxxxx> wrote:
> >
> > David Howells wrote:
> > > David Howells <dhowells@xxxxxxxxxx> 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?
--
Tom Parkin
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development
Attachment:
signature.asc
Description: PGP signature
