Re: [tipc-discussion] [PATCH 1/2] tipc: fix use-after-free tipc_sk_filter_rcv

[Jon M. <jon...@er...>]

Acked-by: jon

> -----Original Message-----
> From: Hoang Le <hoa...@de...>
> Sent: 21-Mar-19 09:29
> To: Jon Maloy <jon...@er...>; ma...@do...;
> yin...@wi...; tip...@li...
> Subject: [PATCH 1/2] tipc: fix use-after-free tipc_sk_filter_rcv
> 
> skb free-ed in:
>   1/ condition 1: tipc_sk_filter_rcv -> tipc_sk_proto_rcv
>   2/ condition 2: tipc_sk_filter_rcv -> tipc_group_filter_msg This leads to a
> "use-after-free" access in the next condition.
> 
> We fix this by intializing the variable at declaration, then it is safe to check this
> variable to continue processing if condition matches.
> 
> syzbot report:
> ==========================================================
> ========
> BUG: KASAN: use-after-free in tipc_sk_filter_rcv+0x2166/0x34f0
>  net/tipc/socket.c:2167
> Read of size 4 at addr ffff88808ea58534 by task kworker/u4:0/7
> 
> CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 5.0.0+ #61 Hardware name:
> Google Google Compute Engine/Google Compute Engine,  BIOS Google
> 01/01/2011
> Workqueue: tipc_send tipc_conn_send_work Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
>  kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
>  __asan_report_load4_noabort+0x14/0x20 mm/kasan/generic_report.c:131
>  tipc_sk_filter_rcv+0x2166/0x34f0 net/tipc/socket.c:2167  tipc_sk_enqueue
> net/tipc/socket.c:2254 [inline]
>  tipc_sk_rcv+0xc45/0x25a0 net/tipc/socket.c:2305
>  tipc_topsrv_kern_evt+0x3b7/0x580 net/tipc/topsrv.c:610
>  tipc_conn_send_to_sock+0x43e/0x5f0 net/tipc/topsrv.c:283
>  tipc_conn_send_work+0x65/0x80 net/tipc/topsrv.c:303
>  process_one_work+0x98e/0x1790 kernel/workqueue.c:2269
>  worker_thread+0x98/0xe40 kernel/workqueue.c:2415
>  kthread+0x357/0x430 kernel/kthread.c:253
>  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
> 
> Reported-by: syz...@sy...
> Fixes: c55c8eda ("tipc: smooth change between replicast and broadcast")
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>  net/tipc/socket.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c index
> 922b75ff56d3..a7b3e1a070e4 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -2151,6 +2151,7 @@ static void tipc_sk_filter_rcv(struct sock *sk, struct
> sk_buff *skb,
>  	struct tipc_msg *hdr = buf_msg(skb);
>  	struct net *net = sock_net(sk);
>  	struct sk_buff_head inputq;
> +	int mtyp = msg_type(hdr);
>  	int limit, err = TIPC_OK;
> 
>  	trace_tipc_sk_filter_rcv(sk, skb, TIPC_DUMP_ALL, " "); @@ -2164,7
> +2165,7 @@ static void tipc_sk_filter_rcv(struct sock *sk, struct sk_buff
> *skb,
>  	if (unlikely(grp))
>  		tipc_group_filter_msg(grp, &inputq, xmitq);
> 
> -	if (msg_type(hdr) == TIPC_MCAST_MSG)
> +	if (unlikely(!grp) && mtyp == TIPC_MCAST_MSG)
>  		tipc_mcast_filter_msg(&tsk->mc_method.deferredq,
> &inputq);
> 
>  	/* Validate and add to receive buffer if there is space */
> --
> 2.1.4