Date: Mon, 8 Apr 2024 15:58:20 +0000
From: Asbjørn Sloth Tønnesen <ast@...erby.net>
To: Jianbo Liu <jianbol@...dia.com>, netdev@...r.kernel.org,
 davem@...emloft.net, marcelo.leitner@...il.com, horms@...nel.org
Cc: Cosmin Ratiu <cratiu@...dia.com>
Subject: Re: [PATCH net-next] net: sched: cls_api: fix slab-use-after-free in
 fl_dump_key

On 4/8/24 1:48 PM, Jianbo Liu wrote:
> The filter counter is updated under the protection of cb_lock in the
> cited commit. While waiting for the lock, it's possible the filter is
> being deleted by other thread, and thus causes UAF when dump it.
> 
> Fix this issue by moving tcf_block_filter_cnt_update() after
> tfilter_put().
> 
>   ==================================================================
>   BUG: KASAN: slab-use-after-free in fl_dump_key+0x1d3e/0x20d0 [cls_flower]
>   Read of size 4 at addr ffff88814f864000 by task tc/2973
> 
>   CPU: 7 PID: 2973 Comm: tc Not tainted 6.9.0-rc2_for_upstream_debug_2024_04_02_12_41 #1
>   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x7e/0xc0
>    print_report+0xc1/0x600
>    ? __virt_addr_valid+0x1cf/0x390
>    ? fl_dump_key+0x1d3e/0x20d0 [cls_flower]
>    ? fl_dump_key+0x1d3e/0x20d0 [cls_flower]
>    kasan_report+0xb9/0xf0
>    ? fl_dump_key+0x1d3e/0x20d0 [cls_flower]
>    fl_dump_key+0x1d3e/0x20d0 [cls_flower]
>    ? lock_acquire+0x1c2/0x530
>    ? fl_dump+0x172/0x5c0 [cls_flower]
>    ? lockdep_hardirqs_on_prepare+0x400/0x400
>    ? fl_dump_key_options.part.0+0x10f0/0x10f0 [cls_flower]
>    ? do_raw_spin_lock+0x12d/0x270
>    ? spin_bug+0x1d0/0x1d0
>    fl_dump+0x21d/0x5c0 [cls_flower]
>    ? fl_tmplt_dump+0x1f0/0x1f0 [cls_flower]
>    ? nla_put+0x15f/0x1c0
>    tcf_fill_node+0x51b/0x9a0
>    ? tc_skb_ext_tc_enable+0x150/0x150
>    ? __alloc_skb+0x17b/0x310
>    ? __build_skb_around+0x340/0x340
>    ? down_write+0x1b0/0x1e0
>    tfilter_notify+0x1a5/0x390
>    ? fl_terse_dump+0x400/0x400 [cls_flower]
>    tc_new_tfilter+0x963/0x2170
>    ? tc_del_tfilter+0x1490/0x1490
>    ? print_usage_bug.part.0+0x670/0x670
>    ? lock_downgrade+0x680/0x680
>    ? security_capable+0x51/0x90
>    ? tc_del_tfilter+0x1490/0x1490
>    rtnetlink_rcv_msg+0x75e/0xac0
>    ? if_nlmsg_stats_size+0x4c0/0x4c0
>    ? lockdep_set_lock_cmp_fn+0x190/0x190
>    ? __netlink_lookup+0x35e/0x6e0
>    netlink_rcv_skb+0x12c/0x360
>    ? if_nlmsg_stats_size+0x4c0/0x4c0
>    ? netlink_ack+0x15e0/0x15e0
>    ? lockdep_hardirqs_on_prepare+0x400/0x400
>    ? netlink_deliver_tap+0xcd/0xa60
>    ? netlink_deliver_tap+0xcd/0xa60
>    ? netlink_deliver_tap+0x1c9/0xa60
>    netlink_unicast+0x43e/0x700
>    ? netlink_attachskb+0x750/0x750
>    ? lock_acquire+0x1c2/0x530
>    ? __might_fault+0xbb/0x170
>    netlink_sendmsg+0x749/0xc10
>    ? netlink_unicast+0x700/0x700
>    ? __might_fault+0xbb/0x170
>    ? netlink_unicast+0x700/0x700
>    __sock_sendmsg+0xc5/0x190
>    ____sys_sendmsg+0x534/0x6b0
>    ? import_iovec+0x7/0x10
>    ? kernel_sendmsg+0x30/0x30
>    ? __copy_msghdr+0x3c0/0x3c0
>    ? entry_SYSCALL_64_after_hwframe+0x46/0x4e
>    ? lock_acquire+0x1c2/0x530
>    ? __virt_addr_valid+0x116/0x390
>    ___sys_sendmsg+0xeb/0x170
>    ? __virt_addr_valid+0x1ca/0x390
>    ? copy_msghdr_from_user+0x110/0x110
>    ? __delete_object+0xb8/0x100
>    ? __virt_addr_valid+0x1cf/0x390
>    ? do_sys_openat2+0x102/0x150
>    ? lockdep_hardirqs_on_prepare+0x284/0x400
>    ? do_sys_openat2+0x102/0x150
>    ? __fget_light+0x53/0x1d0
>    ? sockfd_lookup_light+0x1a/0x150
>    __sys_sendmsg+0xb5/0x140
>    ? __sys_sendmsg_sock+0x20/0x20
>    ? lock_downgrade+0x680/0x680
>    do_syscall_64+0x70/0x140
>    entry_SYSCALL_64_after_hwframe+0x46/0x4e
>   RIP: 0033:0x7f98e3713367
>   Code: 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
>   RSP: 002b:00007ffc74a64608 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>   RAX: ffffffffffffffda RBX: 000000000047eae0 RCX: 00007f98e3713367
>   RDX: 0000000000000000 RSI: 00007ffc74a64670 RDI: 0000000000000003
>   RBP: 0000000000000008 R08: 0000000000000000 R09: 0000000000000000
>   R10: 00007f98e360c5e8 R11: 0000000000000246 R12: 00007ffc74a6a508
>   R13: 00000000660d518d R14: 0000000000484a80 R15: 00007ffc74a6a50b
>    </TASK>
> 
>   Allocated by task 2973:
>    kasan_save_stack+0x20/0x40
>    kasan_save_track+0x10/0x30
>    __kasan_kmalloc+0x77/0x90
>    fl_change+0x27a6/0x4540 [cls_flower]
>    tc_new_tfilter+0x879/0x2170
>    rtnetlink_rcv_msg+0x75e/0xac0
>    netlink_rcv_skb+0x12c/0x360
>    netlink_unicast+0x43e/0x700
>    netlink_sendmsg+0x749/0xc10
>    __sock_sendmsg+0xc5/0x190
>    ____sys_sendmsg+0x534/0x6b0
>    ___sys_sendmsg+0xeb/0x170
>    __sys_sendmsg+0xb5/0x140
>    do_syscall_64+0x70/0x140
>    entry_SYSCALL_64_after_hwframe+0x46/0x4e
> 
>   Freed by task 283:
>    kasan_save_stack+0x20/0x40
>    kasan_save_track+0x10/0x30
>    kasan_save_free_info+0x37/0x50
>    poison_slab_object+0x105/0x190
>    __kasan_slab_free+0x11/0x30
>    kfree+0x111/0x340
>    process_one_work+0x787/0x1490
>    worker_thread+0x586/0xd30
>    kthread+0x2df/0x3b0
>    ret_from_fork+0x2d/0x70
>    ret_from_fork_asm+0x11/0x20
> 
>   Last potentially related work creation:
>    kasan_save_stack+0x20/0x40
>    __kasan_record_aux_stack+0x9b/0xb0
>    insert_work+0x25/0x1b0
>    __queue_work+0x640/0xc90
>    rcu_work_rcufn+0x42/0x70
>    rcu_core+0x6a9/0x1850
>    __do_softirq+0x264/0x88f
> 
>   Second to last potentially related work creation:
>    kasan_save_stack+0x20/0x40
>    __kasan_record_aux_stack+0x9b/0xb0
>    __call_rcu_common.constprop.0+0x6f/0xac0
>    queue_rcu_work+0x56/0x70
>    fl_mask_put+0x20d/0x270 [cls_flower]
>    __fl_delete+0x352/0x6b0 [cls_flower]
>    fl_delete+0x97/0x160 [cls_flower]
>    tc_del_tfilter+0x7d1/0x1490
>    rtnetlink_rcv_msg+0x75e/0xac0
>    netlink_rcv_skb+0x12c/0x360
>    netlink_unicast+0x43e/0x700
>    netlink_sendmsg+0x749/0xc10
>    __sock_sendmsg+0xc5/0x190
>    ____sys_sendmsg+0x534/0x6b0
>    ___sys_sendmsg+0xeb/0x170
>    __sys_sendmsg+0xb5/0x140
>    do_syscall_64+0x70/0x140
>    entry_SYSCALL_64_after_hwframe+0x46/0x4e
> 
> Fixes: 2081fd3445fe ("net: sched: cls_api: add filter counter")
> Signed-off-by: Jianbo Liu <jianbol@...dia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@...dia.com>

Tested-by: Asbjørn Sloth Tønnesen <ast@...erby.net>

> ---
>   net/sched/cls_api.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index db0653993632..17d97bbe890f 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -2400,10 +2400,10 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
>   	err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh,
>   			      flags, extack);
>   	if (err == 0) {
> -		tcf_block_filter_cnt_update(block, &tp->counted, true);
>   		tfilter_notify(net, skb, n, tp, block, q, parent, fh,
>   			       RTM_NEWTFILTER, false, rtnl_held, extack);
>   		tfilter_put(tp, fh);
> +		tcf_block_filter_cnt_update(block, &tp->counted, true);
>   		/* q pointer is NULL for shared blocks */
>   		if (q)
>   			q->flags &= ~TCQ_F_CAN_BYPASS;

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541

Powered by blists - more mailing lists