Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220308000146.534935-1-tadeusz.struk@linaro.org>
 <14626165dad64bbaabed58ba7d59e523@AcuMS.aculab.com> <6155b68c-161b-0745-b303-f7e037b56e28@linaro.org>
 <66463e26-8564-9f58-ce41-9a2843891d1a@kernel.org> <45522c89-a3b4-4b98-232b-9c69470124a3@linaro.org>
 <ff2e1007-5883-5178-6415-326d6ae69c34@kernel.org> <8fdab42f-171f-53d7-8e0e-b29161c0e3e2@linaro.org>
 <CA+FuTSeAL7TsdW4t7=G91n3JLuYehUCnDGH4_rHS=vjm1-Nv9Q@mail.gmail.com> <c7608cf0-fda2-1aa6-b0c1-3d4e0b5cad0e@linaro.org>
In-Reply-To: <c7608cf0-fda2-1aa6-b0c1-3d4e0b5cad0e@linaro.org>
From:   Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date:   Thu, 10 Mar 2022 12:32:49 -0500
Message-ID: <CA+FuTScPUVpyK6WYXrePTg_533VF2wfPww4MOJYa17v0xbLeGQ@mail.gmail.com>
Subject: Re: [PATCH] net: ipv6: fix invalid alloclen in __ip6_append_data
To:     Tadeusz Struk <tadeusz.struk@linaro.org>
Cc:     Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        David Ahern <dsahern@kernel.org>,
        David Laight <David.Laight@aculab.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        Jakub Kicinski <kuba@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        "syzbot+e223cf47ec8ae183f2a0@syzkaller.appspotmail.com" 
        <syzbot+e223cf47ec8ae183f2a0@syzkaller.appspotmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Mar 10, 2022 at 11:06 AM Tadeusz Struk <tadeusz.struk@linaro.org> wrote:
>
> On 3/10/22 06:39, Willem de Bruijn wrote:
> > On Wed, Mar 9, 2022 at 4:37 PM Tadeusz Struk <tadeusz.struk@linaro.org> wrote:
> >>
> >> On 3/8/22 21:01, David Ahern wrote:
> >>> On 3/8/22 12:46 PM, Tadeusz Struk wrote:
> >>>> That fails in the same way:
> >>>>
> >>>> skbuff: skb_over_panic: text:ffffffff83e7b48b len:65575 put:65575
> >>>> head:ffff888101f8a000 data:ffff888101f8a088 tail:0x100af end:0x6c0
> >>>> dev:<NULL>
> >>>> ------------[ cut here ]------------
> >>>> kernel BUG at net/core/skbuff.c:113!
> >>>> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> >>>> CPU: 0 PID: 1852 Comm: repro Not tainted
> >>>> 5.17.0-rc7-00020-gea4424be1688-dirty #19
> >>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1.fc35
> >>>> RIP: 0010:skb_panic+0x173/0x175
> >>>>
> >>>> I'm not sure how it supposed to help since it doesn't change the
> >>>> alloclen at all.
> >>>
> >>> alloclen is a function of fraglen and fraglen is a function of datalen.
> >>
> >> Ok, but in this case it doesn't affect the alloclen and it still fails.
> >
> > This is some kind of non-standard packet that is being constructed. Do
> > we understand how it is different?
> >
> > The .syz reproducer is generally a bit more readable than the .c
> > equivalent. Though not as much as an strace of the binary, if you
> > can share that.
> >
> > r0 = socket$inet6_icmp_raw(0xa, 0x3, 0x3a)
> > connect$inet6(r0, &(0x7f0000000040)={0xa, 0x0, 0x0, @dev, 0x6}, 0x1c)
> > setsockopt$inet6_IPV6_HOPOPTS(r0, 0x29, 0x36,
> > &(0x7f0000000100)=ANY=[@ANYBLOB="52b3"], 0x5a0)
> > sendmmsg$inet(r0, &(0x7f00000002c0)=[{{0x0, 0x0,
> > &(0x7f0000000000)=[{&(0x7f00000000c0)="1d2d", 0xfa5f}], 0x1}}], 0x1,
> > 0xfe80)
>
> Here it is:
> https://termbin.com/krtr
> It won't be of much help, I'm afraid, as the offending sendmmsg()
> call isn't fully printed.

Thanks. It does offer some hints on the other two syscalls:

[pid   644] connect(3, {sa_family=AF_INET6, sin6_port=htons(0),
sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "fe80::", &sin6_addr),
sin6_scope_id=if_nametoindex("tunl0")}, 28) = 0
[pid   644] setsockopt(3, SOL_IPV6, IPV6_HOPOPTS,
"R\263\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
1440) = 0

IPV6_HOPOPTS is ns_capable CAP_NET_RAW.

So this adds 1440 bytes to opt_nflen, which is included in
fragheaderlen, causing that to be exactly mtu. This means that the
payload can never be sent, as each fragment header eats up the entire
mtu? This is without any transport headers that would only be part of
the first fragment (which go into opt_flen).

If you can maybe catch the error before the skb_put and just return
EINVAL, we might see whether sendmmsg is relevant or a simple send
would be equivalent. (not super important, that appears unrelated.)