From f631ef39d81956a2ee69d25039781ceae1162f62 Mon Sep 17 00:00:00 2001 From: Asbjørn Sloth Tønnesen Date: Mon, 25 Mar 2024 20:47:34 +0000 Subject: net: sched: cls_api: add skip_sw counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintain a count of skip_sw filters. This counter is protected by the cb_lock, and is updated at the same time as offloadcnt. Signed-off-by: Asbjørn Sloth Tønnesen Reviewed-by: Jiri Pirko Reviewed-by: Simon Horman Reviewed-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sched/cls_api.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'net/sched/cls_api.c') diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index ca5676b2668e..397c3d29659c 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -3483,6 +3483,8 @@ static void tcf_block_offload_inc(struct tcf_block *block, u32 *flags) if (*flags & TCA_CLS_FLAGS_IN_HW) return; *flags |= TCA_CLS_FLAGS_IN_HW; + if (tc_skip_sw(*flags)) + atomic_inc(&block->skipswcnt); atomic_inc(&block->offloadcnt); } @@ -3491,6 +3493,8 @@ static void tcf_block_offload_dec(struct tcf_block *block, u32 *flags) if (!(*flags & TCA_CLS_FLAGS_IN_HW)) return; *flags &= ~TCA_CLS_FLAGS_IN_HW; + if (tc_skip_sw(*flags)) + atomic_dec(&block->skipswcnt); atomic_dec(&block->offloadcnt); } -- cgit v1.2.3-59-g8ed1b From 2081fd3445fec6b9813c20e8b910c2abd6de31cb Mon Sep 17 00:00:00 2001 From: Asbjørn Sloth Tønnesen Date: Mon, 25 Mar 2024 20:47:35 +0000 Subject: net: sched: cls_api: add filter counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintain a count of filters per block. Counter updates are protected by cb_lock, which is also used to protect the offload counters. Signed-off-by: Asbjørn Sloth Tønnesen Reviewed-by: Simon Horman Reviewed-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- include/net/sch_generic.h | 2 ++ net/sched/cls_api.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) (limited to 'net/sched/cls_api.c') diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 120a4ca6ec9b..eb3872c22fcd 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -422,6 +422,7 @@ struct tcf_proto { */ spinlock_t lock; bool deleting; + bool counted; refcount_t refcnt; struct rcu_head rcu; struct hlist_node destroy_ht_node; @@ -471,6 +472,7 @@ struct tcf_block { struct flow_block flow_block; struct list_head owner_list; bool keep_dst; + atomic_t filtercnt; /* Number of filters */ atomic_t skipswcnt; /* Number of skip_sw filters */ atomic_t offloadcnt; /* Number of oddloaded filters */ unsigned int nooffloaddevcnt; /* Number of devs unable to do offload */ diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 397c3d29659c..304a46ab0e0b 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -410,12 +410,30 @@ static void tcf_proto_get(struct tcf_proto *tp) refcount_inc(&tp->refcnt); } +static void tcf_block_filter_cnt_update(struct tcf_block *block, bool *counted, bool add) +{ + lockdep_assert_not_held(&block->cb_lock); + + down_write(&block->cb_lock); + if (*counted != add) { + if (add) { + atomic_inc(&block->filtercnt); + *counted = true; + } else { + atomic_dec(&block->filtercnt); + *counted = false; + } + } + up_write(&block->cb_lock); +} + static void tcf_chain_put(struct tcf_chain *chain); static void tcf_proto_destroy(struct tcf_proto *tp, bool rtnl_held, bool sig_destroy, struct netlink_ext_ack *extack) { tp->ops->destroy(tp, rtnl_held, extack); + tcf_block_filter_cnt_update(tp->chain->block, &tp->counted, false); if (sig_destroy) tcf_proto_signal_destroyed(tp->chain, tp); tcf_chain_put(tp->chain); @@ -2364,6 +2382,7 @@ replay: 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); -- cgit v1.2.3-59-g8ed1b From 047f340b36fc550c0fc6a8947fc0a1f8e429e9ab Mon Sep 17 00:00:00 2001 From: Asbjørn Sloth Tønnesen Date: Mon, 25 Mar 2024 20:47:36 +0000 Subject: net: sched: make skip_sw actually skip software MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TC filters come in 3 variants: - no flag (try to process in hardware, but fallback to software)) - skip_hw (do not process filter by hardware) - skip_sw (do not process filter by software) However skip_sw is implemented so that the skip_sw flag can first be checked, after it has been matched. IMHO it's common when using skip_sw, to use it on all rules. So if all filters in a block is skip_sw filters, then we can bail early, we can thus avoid having to match the filters, just to check for the skip_sw flag. This patch adds a bypass, for when only TC skip_sw rules are used. The bypass is guarded by a static key, to avoid harming other workloads. There are 3 ways that a packet from a skip_sw ruleset, can end up in the kernel path. Although the send packets to a non-existent chain way is only improved a few percents, then I believe it's worth optimizing the trap and fall-though use-cases. +----------------------------+--------+--------+--------+ | Test description | Pre- | Post- | Rel. | | | kpps | kpps | chg. | +----------------------------+--------+--------+--------+ | basic forwarding + notrack | 3589.3 | 3587.9 | 1.00x | | switch to eswitch mode | 3081.8 | 3094.7 | 1.00x | | add ingress qdisc | 3042.9 | 3063.6 | 1.01x | | tc forward in hw / skip_sw |37024.7 |37028.4 | 1.00x | | tc forward in sw / skip_hw | 3245.0 | 3245.3 | 1.00x | +----------------------------+--------+--------+--------+ | tests with only skip_sw rules below: | +----------------------------+--------+--------+--------+ | 1 non-matching rule | 2694.7 | 3058.7 | 1.14x | | 1 n-m rule, match trap | 2611.2 | 3323.1 | 1.27x | | 1 n-m rule, goto non-chain | 2886.8 | 2945.9 | 1.02x | | 5 non-matching rules | 1958.2 | 3061.3 | 1.56x | | 5 n-m rules, match trap | 1911.9 | 3327.0 | 1.74x | | 5 n-m rules, goto non-chain| 2883.1 | 2947.5 | 1.02x | | 10 non-matching rules | 1466.3 | 3062.8 | 2.09x | | 10 n-m rules, match trap | 1444.3 | 3317.9 | 2.30x | | 10 n-m rules,goto non-chain| 2883.1 | 2939.5 | 1.02x | | 25 non-matching rules | 838.5 | 3058.9 | 3.65x | | 25 n-m rules, match trap | 824.5 | 3323.0 | 4.03x | | 25 n-m rules,goto non-chain| 2875.8 | 2944.7 | 1.02x | | 50 non-matching rules | 488.1 | 3054.7 | 6.26x | | 50 n-m rules, match trap | 484.9 | 3318.5 | 6.84x | | 50 n-m rules,goto non-chain| 2884.1 | 2939.7 | 1.02x | +----------------------------+--------+--------+--------+ perf top (25 n-m skip_sw rules - pre patch): 20.39% [kernel] [k] __skb_flow_dissect 16.43% [kernel] [k] rhashtable_jhash2 10.58% [kernel] [k] fl_classify 10.23% [kernel] [k] fl_mask_lookup 4.79% [kernel] [k] memset_orig 2.58% [kernel] [k] tcf_classify 1.47% [kernel] [k] __x86_indirect_thunk_rax 1.42% [kernel] [k] __dev_queue_xmit 1.36% [kernel] [k] nft_do_chain 1.21% [kernel] [k] __rcu_read_lock perf top (25 n-m skip_sw rules - post patch): 5.12% [kernel] [k] __dev_queue_xmit 4.77% [kernel] [k] nft_do_chain 3.65% [kernel] [k] dev_gro_receive 3.41% [kernel] [k] check_preemption_disabled 3.14% [kernel] [k] mlx5e_skb_from_cqe_mpwrq_nonlinear 2.88% [kernel] [k] __netif_receive_skb_core.constprop.0 2.49% [kernel] [k] mlx5e_xmit 2.15% [kernel] [k] ip_forward 1.95% [kernel] [k] mlx5e_tc_restore_tunnel 1.92% [kernel] [k] vlan_gro_receive Test setup: DUT: Intel Xeon D-1518 (2.20GHz) w/ Nvidia/Mellanox ConnectX-6 Dx 2x100G Data rate measured on switch (Extreme X690), and DUT connected as a router on a stick, with pktgen and pktsink as VLANs. Pktgen-dpdk was in range 36.6-37.7 Mpps 64B packets across all tests. Full test data at https://files.fiberby.net/ast/2024/tc_skip_sw/v2_tests/ Signed-off-by: Asbjørn Sloth Tønnesen Reviewed-by: Simon Horman Reviewed-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 9 +++++++++ include/net/sch_generic.h | 1 + net/core/dev.c | 10 ++++++++++ net/sched/cls_api.c | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+) (limited to 'net/sched/cls_api.c') diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index a4ee43f493bb..41297bd38dff 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -74,6 +74,15 @@ static inline bool tcf_block_non_null_shared(struct tcf_block *block) return block && block->index; } +#ifdef CONFIG_NET_CLS_ACT +DECLARE_STATIC_KEY_FALSE(tcf_bypass_check_needed_key); + +static inline bool tcf_block_bypass_sw(struct tcf_block *block) +{ + return block && block->bypass_wanted; +} +#endif + static inline struct Qdisc *tcf_block_q(struct tcf_block *block) { WARN_ON(tcf_block_shared(block)); diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index eb3872c22fcd..76db6be16083 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -472,6 +472,7 @@ struct tcf_block { struct flow_block flow_block; struct list_head owner_list; bool keep_dst; + bool bypass_wanted; atomic_t filtercnt; /* Number of filters */ atomic_t skipswcnt; /* Number of skip_sw filters */ atomic_t offloadcnt; /* Number of oddloaded filters */ diff --git a/net/core/dev.c b/net/core/dev.c index 5d36a634f468..c136e80dea61 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2083,6 +2083,11 @@ void net_dec_egress_queue(void) EXPORT_SYMBOL_GPL(net_dec_egress_queue); #endif +#ifdef CONFIG_NET_CLS_ACT +DEFINE_STATIC_KEY_FALSE(tcf_bypass_check_needed_key); +EXPORT_SYMBOL(tcf_bypass_check_needed_key); +#endif + DEFINE_STATIC_KEY_FALSE(netstamp_needed_key); EXPORT_SYMBOL(netstamp_needed_key); #ifdef CONFIG_JUMP_LABEL @@ -3937,6 +3942,11 @@ static int tc_run(struct tcx_entry *entry, struct sk_buff *skb, if (!miniq) return ret; + if (static_branch_unlikely(&tcf_bypass_check_needed_key)) { + if (tcf_block_bypass_sw(miniq->block)) + return ret; + } + tc_skb_cb(skb)->mru = 0; tc_skb_cb(skb)->post_ct = false; tcf_set_drop_reason(skb, *drop_reason); diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 304a46ab0e0b..db0653993632 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -410,6 +410,23 @@ static void tcf_proto_get(struct tcf_proto *tp) refcount_inc(&tp->refcnt); } +static void tcf_maintain_bypass(struct tcf_block *block) +{ + int filtercnt = atomic_read(&block->filtercnt); + int skipswcnt = atomic_read(&block->skipswcnt); + bool bypass_wanted = filtercnt > 0 && filtercnt == skipswcnt; + + if (bypass_wanted != block->bypass_wanted) { +#ifdef CONFIG_NET_CLS_ACT + if (bypass_wanted) + static_branch_inc(&tcf_bypass_check_needed_key); + else + static_branch_dec(&tcf_bypass_check_needed_key); +#endif + block->bypass_wanted = bypass_wanted; + } +} + static void tcf_block_filter_cnt_update(struct tcf_block *block, bool *counted, bool add) { lockdep_assert_not_held(&block->cb_lock); @@ -424,6 +441,7 @@ static void tcf_block_filter_cnt_update(struct tcf_block *block, bool *counted, *counted = false; } } + tcf_maintain_bypass(block); up_write(&block->cb_lock); } -- cgit v1.2.3-59-g8ed1b From 2ecd487b670fcbb1ad4893fff1af4aafdecb6023 Mon Sep 17 00:00:00 2001 From: Jianbo Liu Date: Mon, 8 Apr 2024 16:48:17 +0300 Subject: net: sched: cls_api: fix slab-use-after-free in fl_dump_key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 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 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 Reviewed-by: Cosmin Ratiu Tested-by: Asbjørn Sloth Tønnesen Signed-off-by: David S. Miller --- net/sched/cls_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sched/cls_api.c') 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 @@ replay: 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; -- cgit v1.2.3-59-g8ed1b