Message ID | 20240408134817.35065-1-jianbol@nvidia.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 2ecd487b670fcbb1ad4893fff1af4aafdecb6023 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net-next] net: sched: cls_api: fix slab-use-after-free in fl_dump_key | expand |
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@nvidia.com> > Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> Tested-by: Asbjørn Sloth Tønnesen <ast@fiberby.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;
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Mon, 8 Apr 2024 16:48:17 +0300 you 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(). > > [...] Here is the summary with links: - [net-next] net: sched: cls_api: fix slab-use-after-free in fl_dump_key https://git.kernel.org/netdev/net-next/c/2ecd487b670f You are awesome, thank you!
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;