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

[Hoang H. Le <hoa...@de...>]

Hi Jon, Ying,

Just remind in case you guys missed this email thread.

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