From: John Hurley john.hurley@netronome.com

mainline inclusion from mainline-v5.3-rc1 commit 720f22fed81bc6fd1765db7014651b6718887bea category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I64END CVE: CVE-2022-4269

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h...

--------------------------------

The TC_ACT_REINSERT return type was added as an in-kernel only option to allow a packet ingress or egress redirect. This is used to avoid unnecessary skb clones in situations where they are not required. If a TC hook returns this code then the packet is 'reinserted' and no skb consume is carried out as no clone took place.

This return type is only used in act_mirred. Rather than have the reinsert called from the main datapath, call it directly in act_mirred. Instead of returning TC_ACT_REINSERT, change the type to the new TC_ACT_CONSUMED which tells the caller that the packet has been stolen by another process and that no consume call is required.

Moving all redirect calls to the act_mirred code is in preparation for tracking recursion created by act_mirred.

Signed-off-by: John Hurley john.hurley@netronome.com Reviewed-by: Simon Horman simon.horman@netronome.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Ziyang Xuan william.xuanziyang@huawei.com Reviewed-by: Liu Jian liujian56@huawei.com Reviewed-by: Wang Weiyang wangweiyang2@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- include/net/pkt_cls.h | 2 +- include/net/sch_generic.h | 2 +- net/core/dev.c | 4 +--- net/sched/act_mirred.c | 3 ++- 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 75a3f3fdb359..5d381f2af64b 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -8,7 +8,7 @@ #include <net/act_api.h>

/* TC action not accessible from user space */ -#define TC_ACT_REINSERT (TC_ACT_VALUE_MAX + 1) +#define TC_ACT_CONSUMED (TC_ACT_VALUE_MAX + 1)

/* Basic packet classifier frontend definitions. */

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index f5f45390828d..101481ad5270 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -258,7 +258,7 @@ struct tcf_result { }; const struct tcf_proto *goto_tp;

- /* used by the TC_ACT_REINSERT action */ + /* used in the skb_tc_reinsert function */ struct { bool ingress; struct gnet_stats_queue *qstats; diff --git a/net/core/dev.c b/net/core/dev.c index 09b5b73ecdd8..f1af4712a78b 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4662,9 +4662,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret, __skb_push(skb, skb->mac_len); skb_do_redirect(skb); return NULL; - case TC_ACT_REINSERT: - /* this does not scrub the packet, and updates stats on error */ - skb_tc_reinsert(skb, &cl_res); + case TC_ACT_CONSUMED: return NULL; default: break; diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 9ee7c7f479fc..55683fc96ad3 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -269,7 +269,8 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a, if (use_reinsert) { res->ingress = want_ingress; res->qstats = this_cpu_ptr(m->common.cpu_qstats); - return TC_ACT_REINSERT; + skb_tc_reinsert(skb, res); + return TC_ACT_CONSUMED; } }

From: Vlad Buslov vladbu@mellanox.com

mainline inclusion from mainline-v5.5-rc1 commit ef816f3c49c1c404ababc50e10d4cbe5109da678 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I64END CVE: CVE-2022-4269

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h...

--------------------------------

Previous commit introduced helper function for updating qstats and refactored set of actions to use the helpers, instead of modifying qstats directly. However, one of the affected action exposes its qstats to skb_tc_reinsert(), which then modifies it.

Refactor skb_tc_reinsert() to return integer error code and don't increment overlimit qstats in case of error, and use the returned error code in tcf_mirred_act() to manually increment the overlimit counter with new helper function.

Signed-off-by: Vlad Buslov vladbu@mellanox.com Acked-by: Jiri Pirko jiri@mellanox.com Signed-off-by: David S. Miller davem@davemloft.net Conflicts: net/sched/act_mirred.c Signed-off-by: Ziyang Xuan william.xuanziyang@huawei.com Reviewed-by: Liu Jian liujian56@huawei.com Reviewed-by: Wang Weiyang wangweiyang2@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- include/net/sch_generic.h | 12 ++---------- net/sched/act_mirred.c | 3 ++- 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 101481ad5270..8df77847b6f5 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -1142,17 +1142,9 @@ void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp, void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc, struct mini_Qdisc __rcu **p_miniq);

-static inline void skb_tc_reinsert(struct sk_buff *skb, struct tcf_result *res) +static inline int skb_tc_reinsert(struct sk_buff *skb, struct tcf_result *res) { - struct gnet_stats_queue *stats = res->qstats; - int ret; - - if (res->ingress) - ret = netif_receive_skb(skb); - else - ret = dev_queue_xmit(skb); - if (ret && stats) - qstats_overlimit_inc(res->qstats); + return res->ingress ? netif_receive_skb(skb) : dev_queue_xmit(skb); }

#endif diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 9c18967913c7..c9f184499d4f 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -281,7 +281,8 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a, if (use_reinsert) { res->ingress = want_ingress; res->qstats = this_cpu_ptr(m->common.cpu_qstats); - skb_tc_reinsert(skb, res); + if (skb_tc_reinsert(skb, res) && res->qstats) + qstats_overlimit_inc(res->qstats); __this_cpu_dec(mirred_rec_level); return TC_ACT_CONSUMED; }

From: Davide Caratti dcaratti@redhat.com

mainline inclusion from mainline-v6.3-rc1 commit ca22da2fbd693b54dc8e3b7b54ccc9f7e9ba3640 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I64END CVE: CVE-2022-4269

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h...

--------------------------------

William reports kernel soft-lockups on some OVS topologies when TC mirred egress->ingress action is hit by local TCP traffic [1]. The same can also be reproduced with SCTP (thanks Xin for verifying), when client and server reach themselves through mirred egress to ingress, and one of the two peers sends a "heartbeat" packet (from within a timer).

Enqueueing to backlog proved to fix this soft lockup; however, as Cong noticed [2], we should preserve - when possible - the current mirred behavior that counts as "overlimits" any eventual packet drop subsequent to the mirred forwarding action [3]. A compromise solution might use the backlog only when tcf_mirred_act() has a nest level greater than one: change tcf_mirred_forward() accordingly.

Also, add a kselftest that can reproduce the lockup and verifies TC mirred ability to account for further packet drops after TC mirred egress->ingress (when the nest level is 1).

[1] https://lore.kernel.org/netdev/33dc43f587ec1388ba456b4915c75f02a8aae226.1663... [2] https://lore.kernel.org/netdev/Y0w%2FWWY60gqrtGLp@pop-os.localdomain/ [3] such behavior is not guaranteed: for example, if RPS or skb RX timestamping is enabled on the mirred target device, the kernel can defer receiving the skb and return NET_RX_SUCCESS inside tcf_mirred_forward().

Reported-by: William Zhao wizhao@redhat.com CC: Xin Long lucien.xin@gmail.com Signed-off-by: Davide Caratti dcaratti@redhat.com Reviewed-by: Marcelo Ricardo Leitner marcelo.leitner@gmail.com Acked-by: Jamal Hadi Salim jhs@mojatatu.com Signed-off-by: Paolo Abeni pabeni@redhat.com Conflicts: net/sched/act_mirred.c tools/testing/selftests/net/forwarding/tc_actions.sh Signed-off-by: Ziyang Xuan william.xuanziyang@huawei.com Reviewed-by: Liu Jian liujian56@huawei.com Reviewed-by: Wang Weiyang wangweiyang2@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- net/sched/act_mirred.c | 7 +++ .../selftests/net/forwarding/tc_actions.sh | 49 ++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index f4b28785695c..febf06b8bbdf 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -197,12 +197,19 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla, return ret; }

+static bool is_mirred_nested(void) +{ + return unlikely(__this_cpu_read(mirred_rec_level) > 1); +} + static int tcf_mirred_forward(bool want_ingress, struct sk_buff *skb) { int err;

if (!want_ingress) err = dev_queue_xmit(skb); + else if (is_mirred_nested()) + err = netif_rx(skb); else err = netif_receive_skb(skb);

diff --git a/tools/testing/selftests/net/forwarding/tc_actions.sh b/tools/testing/selftests/net/forwarding/tc_actions.sh index 813d02d1939d..aaa1ea10ac83 100755 --- a/tools/testing/selftests/net/forwarding/tc_actions.sh +++ b/tools/testing/selftests/net/forwarding/tc_actions.sh @@ -2,7 +2,8 @@ # SPDX-License-Identifier: GPL-2.0

ALL_TESTS="gact_drop_and_ok_test mirred_egress_redirect_test \ - mirred_egress_mirror_test gact_trap_test" + mirred_egress_mirror_test gact_trap_test \ + mirred_egress_to_ingress_tcp_test" NUM_NETIFS=4 source tc_common.sh source lib.sh @@ -148,6 +149,52 @@ gact_trap_test() log_test "trap ($tcflags)" }

+mirred_egress_to_ingress_tcp_test() +{ + local tmpfile=$(mktemp) tmpfile1=$(mktemp) + + RET=0 + dd conv=sparse status=none if=/dev/zero bs=1M count=2 of=$tmpfile + tc filter add dev $h1 protocol ip pref 100 handle 100 egress flower \ + $tcflags ip_proto tcp src_ip 192.0.2.1 dst_ip 192.0.2.2 \ + action ct commit nat src addr 192.0.2.2 pipe \ + action ct clear pipe \ + action ct commit nat dst addr 192.0.2.1 pipe \ + action ct clear pipe \ + action skbedit ptype host pipe \ + action mirred ingress redirect dev $h1 + tc filter add dev $h1 protocol ip pref 101 handle 101 egress flower \ + $tcflags ip_proto icmp \ + action mirred ingress redirect dev $h1 + tc filter add dev $h1 protocol ip pref 102 handle 102 ingress flower \ + ip_proto icmp \ + action drop + + ip vrf exec v$h1 nc --recv-only -w10 -l -p 12345 -o $tmpfile1 & + local rpid=$! + ip vrf exec v$h1 nc -w1 --send-only 192.0.2.2 12345 <$tmpfile + wait -n $rpid + cmp -s $tmpfile $tmpfile1 + check_err $? "server output check failed" + + $MZ $h1 -c 10 -p 64 -a $h1mac -b $h1mac -A 192.0.2.1 -B 192.0.2.1 \ + -t icmp "ping,id=42,seq=5" -q + tc_check_packets "dev $h1 egress" 101 10 + check_err $? "didn't mirred redirect ICMP" + tc_check_packets "dev $h1 ingress" 102 10 + check_err $? "didn't drop mirred ICMP" + local overlimits=$(tc_rule_stats_get ${h1} 101 egress .overlimits) + test ${overlimits} = 10 + check_err $? "wrong overlimits, expected 10 got ${overlimits}" + + tc filter del dev $h1 egress protocol ip pref 100 handle 100 flower + tc filter del dev $h1 egress protocol ip pref 101 handle 101 flower + tc filter del dev $h1 ingress protocol ip pref 102 handle 102 flower + + rm -f $tmpfile $tmpfile1 + log_test "mirred_egress_to_ingress_tcp ($tcflags)" +} + setup_prepare() { h1=${NETIFS[p1]}