Re: [tipc-discussion] [PATCH] tipc: fix use-after-free Read in tipc_named_reinit

[Jon M. <jm...@re...>]

On 6/13/22 00:00, Hoang Huu Le wrote:
> Hi Jon, Ying,
>
> Just remind in case you guys missed this email thread.

Yes, I had missed it. It looks good to me.
///jon

>
> Thanks,
> Hoang
>> -----Original Message-----
>> From: Hoang Le <hoa...@de...>
>> Sent: Tuesday, June 7, 2022 2:35 PM
>> To: jm...@re...; ma...@do...; yin...@wi...; Tung Quang Nguyen <tun...@de...>;
>> tip...@li...
>> Cc: syz...@sy...
>> Subject: [tipc-discussion] [PATCH] tipc: fix use-after-free Read in tipc_named_reinit
>>
>> syzbot found the following issue on:
>> ==================================================================
>> BUG: KASAN: use-after-free in tipc_named_reinit+0x94f/0x9b0
>> net/tipc/name_distr.c:413
>> Read of size 8 at addr ffff88805299a000 by task kworker/1:9/23764
>>
>> CPU: 1 PID: 23764 Comm: kworker/1:9 Not tainted
>> 5.18.0-rc4-syzkaller-00878-g17d49e6e8012 #0
>> Hardware name: Google Compute Engine/Google Compute Engine,
>> BIOS Google 01/01/2011
>> Workqueue: events tipc_net_finalize_work
>> Call Trace:
>>   <TASK>
>>   __dump_stack lib/dump_stack.c:88 [inline]
>>   dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
>>   print_address_description.constprop.0.cold+0xeb/0x495
>> mm/kasan/report.c:313
>>   print_report mm/kasan/report.c:429 [inline]
>>   kasan_report.cold+0xf4/0x1c6 mm/kasan/report.c:491
>>   tipc_named_reinit+0x94f/0x9b0 net/tipc/name_distr.c:413
>>   tipc_net_finalize+0x234/0x3d0 net/tipc/net.c:138
>>   process_one_work+0x996/0x1610 kernel/workqueue.c:2289
>>   worker_thread+0x665/0x1080 kernel/workqueue.c:2436
>>   kthread+0x2e9/0x3a0 kernel/kthread.c:376
>>   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298
>>   </TASK>
>> [...]
>> ==================================================================
>>
>> In the commit
>> d966ddcc3821 ("tipc: fix a deadlock when flushing scheduled work"),
>> the cancel_work_sync() function just to make sure ONLY the work
>> tipc_net_finalize_work() is executing/pending on any CPU completed before
>> tipc namespace is destroyed through tipc_exit_net(). But this function
>> is not guaranteed the work is the last queued. So, the destroyed instance
>> may be accessed in the work which will try to enqueue later.
>>
>> In order to completely fix, we re-order the calling of cancel_work_sync()
>> to make sure the work tipc_net_finalize_work() was last queued and it
>> must be completed by calling cancel_work_sync().
>>
>> Reported-by: syz...@sy...
>> Fixes: d966ddcc3821 ("tipc: fix a deadlock when flushing scheduled work")
>> Signed-off-by: Ying Xue <yin...@wi...>
>> Signed-off-by: Hoang Le <hoa...@de...>
>> ---
>>   net/tipc/core.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/net/tipc/core.c b/net/tipc/core.c
>> index 3f4542e0f065..434e70eabe08 100644
>> --- a/net/tipc/core.c
>> +++ b/net/tipc/core.c
>> @@ -109,10 +109,9 @@ static void __net_exit tipc_exit_net(struct net *net)
>>   	struct tipc_net *tn = tipc_net(net);
>>
>>   	tipc_detach_loopback(net);
>> +	tipc_net_stop(net);
>>   	/* Make sure the tipc_net_finalize_work() finished */
>>   	cancel_work_sync(&tn->work);
>> -	tipc_net_stop(net);
>> -
>>   	tipc_bcast_stop(net);
>>   	tipc_nametbl_stop(net);
>>   	tipc_sk_rht_destroy(net);
>> --
>> 2.30.2
>>
>>
>>
>> _______________________________________________
>> tipc-discussion mailing list
>> tip...@li...
>> https://lists.sourceforge.net/lists/listinfo/tipc-discussion