Re: KASAN: use-after-free Read in tipc_group_is_open

[Cong Wang]

On Tue, Jan 16, 2018 at 5:23 AM, Jon Maloy <jon.maloy@xxxxxxxxxxxx> wrote:
>
>
>> -----Original Message-----
>> From: Cong Wang [mailto:xiyou.wangcong@xxxxxxxxx]
>> Sent: Monday, January 15, 2018 23:44
>> To: syzbot <syzbot+799dafde0286795858ac@xxxxxxxxxxxxxxxxxxxxxxxxx>
>> Cc: David Miller <davem@xxxxxxxxxxxxx>; Jon Maloy
>> <jon.maloy@xxxxxxxxxxxx>; LKML <linux-kernel@xxxxxxxxxxxxxxx>; Linux
>> Kernel Network Developers <netdev@xxxxxxxxxxxxxxx>; syzkaller-
>> bugs@xxxxxxxxxxxxxxxx; tipc-discussion@xxxxxxxxxxxxxxxxxxxxx; Ying Xue
>> <ying.xue@xxxxxxxxxxxxx>
>> Subject: Re: KASAN: use-after-free Read in tipc_group_is_open
>>
>> On Mon, Jan 15, 2018 at 7:58 PM, syzbot
>> <syzbot+799dafde0286795858ac@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>> > Hello,
>> >
>> > syzkaller hit the following crash on
>> > 594831a8aba3fd045c3212a3e3bb9788c77b989d
>> > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/maste
>> > r
>> > compiler: gcc (GCC) 7.1.1 20170620
>> > .config is attached
>> > Raw console output is attached.
>> > C reproducer is attached
>> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ for
>> > information about syzkaller reproducers
>> >
>> >
>> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> > Reported-by: syzbot+799dafde0286795858ac@xxxxxxxxxxxxxxxxxxxxxxxxx
>> > It will help syzbot understand when the bug is fixed. See footer for
>> > details.
>> > If you forward the report, please keep this part and the footer.
>> >
>> >
>> ==========================================================
>> ========
>> > BUG: KASAN: use-after-free in tipc_group_is_open+0x3a/0x40
>> > net/tipc/group.c:106
>> > Read of size 1 at addr ffff8801d89f7378 by task syzkaller275009/3704
>> >
>> > CPU: 0 PID: 3704 Comm: syzkaller275009 Not tainted 4.15.0-rc7+ #190
>> > Hardware name: Google Google Compute Engine/Google Compute Engine,
>> > BIOS Google 01/01/2011 Call Trace:
>> >  __dump_stack lib/dump_stack.c:17 [inline]
>> >  dump_stack+0x194/0x257 lib/dump_stack.c:53
>> >  print_address_description+0x73/0x250 mm/kasan/report.c:252
>> > kasan_report_error mm/kasan/report.c:351 [inline]
>> >  kasan_report+0x25b/0x340 mm/kasan/report.c:409
>> >  __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
>> >  tipc_group_is_open+0x3a/0x40 net/tipc/group.c:106
>> >  tipc_poll+0x364/0x4d0 net/tipc/socket.c:740
>>
>>
>> commit eb929a91b213d2a72c5a8b4af9a1acf63bfb8287
>> Author: Jon Maloy <jon.maloy@xxxxxxxxxxxx>
>> Date:   Mon Jan 8 21:03:31 2018 +0100
>>
>>     tipc: improve poll() for group member socket
>>
>> Apparently Jon's commit doesn't fix this. I also don't understand why Jon
>> believes sock_poll_wait() could sync with setsockopt path...
>
> While sock_poll_wait() is sleeping, it is possible that the item the 'grp' stack variable is pointing to might be deleted, invalidating the pointer.
> This is why I moved the initialization of the pointer to after sock_poll_wait().

This doesn't matter at all as long as it doesn't sync setsockopt() path.


> However, now tipc_sock->group is clearly set to NULL at both locations where a group item might be deleted, so the reason for the warning must be something else.
> I am open to suggestions.

Another thread calling  tipc_sk_join() could jump in at any time
since we don't have the sock locked here. This is why I said
we probably need to lock the sock here, unless of course
you can refactor the logic to make tipc_poll() not to touch
tsk->group.