[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89i+wUq5R2nFO8eGLp7=8Y5OiJ0fwjR+ES74gk1X4k9r0rw@mail.gmail.com>
Date: Thu, 21 Sep 2023 13:09:21 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: David Howells <dhowells@...hat.com>
Cc: netdev@...r.kernel.org,
syzbot+62cbf263225ae13ff153@...kaller.appspotmail.com,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
"David S. Miller" <davem@...emloft.net>,
David Ahern <dsahern@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>, bpf@...r.kernel.org,
syzkaller-bugs@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v3] ipv4, ipv6: Fix handling of transhdrlen in __ip{,6}_append_data()
On Thu, Sep 21, 2023 at 12:41 PM David Howells <dhowells@...hat.com> wrote:
>
>
> Including the transhdrlen in length is a problem when the packet is
> partially filled (e.g. something like send(MSG_MORE) happened previously)
> when appending to an IPv4 or IPv6 packet as we don't want to repeat the
> transport header or account for it twice. This can happen under some
> circumstances, such as splicing into an L2TP socket.
>
> The symptom observed is a warning in __ip6_append_data():
>
> WARNING: CPU: 1 PID: 5042 at net/ipv6/ip6_output.c:1800 __ip6_append_data.isra.0+0x1be8/0x47f0 net/ipv6/ip6_output.c:1800
>
> that occurs when MSG_SPLICE_PAGES is used to append more data to an already
> partially occupied skbuff. The warning occurs when 'copy' is larger than
> the amount of data in the message iterator. This is because the requested
> length includes the transport header length when it shouldn't. This can be
> triggered by, for example:
>
> sfd = socket(AF_INET6, SOCK_DGRAM, IPPROTO_L2TP);
> bind(sfd, ...); // ::1
> connect(sfd, ...); // ::1 port 7
> send(sfd, buffer, 4100, MSG_MORE);
> sendfile(sfd, dfd, NULL, 1024);
>
> Fix this by only adding transhdrlen into the length if the write queue is
> empty in l2tp_ip6_sendmsg(), analogously to how UDP does things.
>
> l2tp_ip_sendmsg() looks like it won't suffer from this problem as it builds
> the UDP packet itself.
>
> Fixes: a32e0eec7042 ("l2tp: introduce L2TPv3 IP encapsulation support for IPv6")
> Reported-by: syzbot+62cbf263225ae13ff153@...kaller.appspotmail.com
> Link: https://lore.kernel.org/r/0000000000001c12b30605378ce8@google.com/
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@...il.com>
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Eric Dumazet <edumazet@...gle.com>
> cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>
> cc: "David S. Miller" <davem@...emloft.net>
> cc: David Ahern <dsahern@...nel.org>
> cc: Paolo Abeni <pabeni@...hat.com>
> cc: Jakub Kicinski <kuba@...nel.org>
> cc: netdev@...r.kernel.org
> cc: bpf@...r.kernel.org
> cc: syzkaller-bugs@...glegroups.com
> ---
Looks safer indeed, thanks to you and Willem !
Reviewed-by: Eric Dumazet <edumazet@...gle.com>
Powered by blists - more mailing lists