netdev - Re: [PATCH net-next] ipv6: add debug checks in fib6_info

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

Date: Thu, 7 Dec 2023 19:55:08 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Kui-Feng Lee <sinquersw@...il.com>
Cc: patchwork-bot+netdevbpf@...nel.org, Kui-Feng Lee <thinker.li@...il.com>, 
	davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, dsahern@...nel.org, 
	netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next] ipv6: add debug checks in fib6_info_release()

On Thu, Dec 7, 2023 at 7:46 PM Kui-Feng Lee <sinquersw@...il.com> wrote:
>
>
>
> On 12/7/23 10:22, Eric Dumazet wrote:
> > On Thu, Dec 7, 2023 at 7:19 PM Kui-Feng Lee <sinquersw@...il.com> wrote:
> >>
> >>
> >>
> >> On 12/7/23 10:10, Eric Dumazet wrote:
> >>> On Thu, Dec 7, 2023 at 7:06 PM Kui-Feng Lee <sinquersw@...il.com> wrote:
> >>>
> >>>> Do you happen to have a test program that can reproduce it?
> >>>
> >>> syzbot has a repro, let me release the bug.
> >>>
> >>> Of course syzbot bisection points to my last patch.
> >>
> >> I just looked into the code.
> >> The origin issue mentioned at the thread head should be something
> >> related to a GC change I made. But, the warnings you added doesn't
> >> catch the the error correctly.  According to your stacktrace
> >>
> >>
> >>   > ip6_route_add+0x26/0x1f0 net/ipv6/route.c:3843
> >>   > ipv6_route_ioctl+0x3ff/0x590 net/ipv6/route.c:4467
> >>   > inet6_ioctl+0x265/0x2b0 net/ipv6/af_inet6.c:575
> >>   > sock_do_ioctl+0x113/0x270 net/socket.c:1220
> >>   > sock_ioctl+0x22e/0x6b0 net/socket.c:1339
> >>   > vfs_ioctl fs/ioctl.c:51 [inline]
> >>   > __do_sys_ioctl fs/ioctl.c:871 [inline]
> >>   > __se_sys_ioctl fs/ioctl.c:857 [inline]
> >>   > __x64_sys_ioctl+0x18f/0x210 fs/ioctl.c:857
> >>   > do_syscall_x64 arch/x86/entry/common.c:51 [inline]
> >>   > do_syscall_64+0x40/0x110 arch/x86/entry/common.c:82
> >>   > entry_SYSCALL_64_after_hwframe+0x63/0x6b
> >>
> >> and warning messages you provided
> >>
> >>   > WARNING: CPU: 0 PID: 5059 at include/net/ip6_fib.h:332
> >>   > fib6_info_release include/net/ip6_fib.h:332 [inline]
> >>   > WARNING: CPU: 0 PID: 5059 at include/net/ip6_fib.h:332
> >>   > ip6_route_info_create+0x1a1a/0x1f10 net/ipv6/route.c:3829
> >>
> >> It takes place in ip6_route_info_create() to do error handling.
> >> It can be fib6_has_expires() in fib6_info_release() in this case.
> >
> > Feel free to amend the patch, but the issue is that we insert a fib
> > gc_link to a list,
> > then free the fi6 object without removing it first from the external list.
> >
> > I added two different warnings, and removing one or both will still
> > keep the bug.
>
> The gc_link is not inserted here actually. (see my explanation in
> another message.)
>
> According to the messages in the thread head, it is an issue of dangling
> pointer, right? If I read it correctly, the original issue is gc_link
> pointing to a block of memory that is already free. Am I right?

Original issue is about gc_link corruption, or use-after-free

Perhaps fib6_has_expires()  should really be rewritten to use
hlist_unhashed(&f6i->gc_link)

Setting RTF_EXPIRES on f6i->fib6_flags seems redundant/risky in light
of all the syzbot reports I had lately...