* [PATCH 5.10 0/3] Fix for CVE-2022-4269
@ 2023-05-16 19:00 Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 1/3] net/sched: act_mirred: refactor the handle of xmit Dragos-Marian Panait
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Dragos-Marian Panait @ 2023-05-16 19:00 UTC (permalink / raw)
To: stable
Cc: wenxu, Jakub Kicinski, Jamal Hadi Salim, Davide Caratti,
Marcelo Ricardo Leitner, Paolo Abeni, William Zhao, Xin Long,
David S . Miller, Eric Dumazet, Cong Wang, Jiri Pirko, Shuah Khan,
linux-kselftest, netdev
The following commits are needed to fix CVE-2022-4269:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fa6d639930ee5cd3f932cc314f3407f07a06582d
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=78dcdffe0418ac8f3f057f26fe71ccf4d8ed851f
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ca22da2fbd693b54dc8e3b7b54ccc9f7e9ba3640
Davide Caratti (2):
net/sched: act_mirred: better wording on protection against excessive
stack growth
act_mirred: use the backlog for nested calls to mirred ingress
wenxu (1):
net/sched: act_mirred: refactor the handle of xmit
include/net/sch_generic.h | 5 --
net/sched/act_mirred.c | 44 +++++++++++------
.../selftests/net/forwarding/tc_actions.sh | 48 ++++++++++++++++++-
3 files changed, 77 insertions(+), 20 deletions(-)
base-commit: f1b32fda06d2cfb8eea9680b0ba7a8b0d5b81eeb
--
2.40.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 5.10 1/3] net/sched: act_mirred: refactor the handle of xmit
2023-05-16 19:00 [PATCH 5.10 0/3] Fix for CVE-2022-4269 Dragos-Marian Panait
@ 2023-05-16 19:00 ` Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 2/3] net/sched: act_mirred: better wording on protection against excessive stack growth Dragos-Marian Panait
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Dragos-Marian Panait @ 2023-05-16 19:00 UTC (permalink / raw)
To: stable
Cc: wenxu, Jakub Kicinski, Jamal Hadi Salim, Davide Caratti,
Marcelo Ricardo Leitner, Paolo Abeni, William Zhao, Xin Long,
David S . Miller, Eric Dumazet, Cong Wang, Jiri Pirko, Shuah Khan,
linux-kselftest, netdev
From: wenxu <wenxu@ucloud.cn>
[ Upstream commit fa6d639930ee5cd3f932cc314f3407f07a06582d ]
This one is prepare for the next patch.
Signed-off-by: wenxu <wenxu@ucloud.cn>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[DP: adjusted context for linux-5.10.y]
Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
---
include/net/sch_generic.h | 5 -----
net/sched/act_mirred.c | 21 +++++++++++++++------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 61cd19ee51f4..a62677be7452 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -1320,11 +1320,6 @@ void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
void mini_qdisc_pair_block_init(struct mini_Qdisc_pair *miniqp,
struct tcf_block *block);
-static inline int skb_tc_reinsert(struct sk_buff *skb, struct tcf_result *res)
-{
- return res->ingress ? netif_receive_skb(skb) : dev_queue_xmit(skb);
-}
-
/* Make sure qdisc is no longer in SCHED state. */
static inline void qdisc_synchronize(const struct Qdisc *q)
{
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 24d561d8d9c9..53594b0466eb 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -206,6 +206,18 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
return err;
}
+static int tcf_mirred_forward(bool want_ingress, struct sk_buff *skb)
+{
+ int err;
+
+ if (!want_ingress)
+ err = dev_queue_xmit(skb);
+ else
+ err = netif_receive_skb(skb);
+
+ return err;
+}
+
static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
struct tcf_result *res)
{
@@ -295,18 +307,15 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
/* let's the caller reinsert the packet, if possible */
if (use_reinsert) {
res->ingress = want_ingress;
- if (skb_tc_reinsert(skb, res))
+ err = tcf_mirred_forward(res->ingress, skb);
+ if (err)
tcf_action_inc_overlimit_qstats(&m->common);
__this_cpu_dec(mirred_rec_level);
return TC_ACT_CONSUMED;
}
}
- if (!want_ingress)
- err = dev_queue_xmit(skb2);
- else
- err = netif_receive_skb(skb2);
-
+ err = tcf_mirred_forward(want_ingress, skb2);
if (err) {
out:
tcf_action_inc_overlimit_qstats(&m->common);
--
2.40.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 5.10 2/3] net/sched: act_mirred: better wording on protection against excessive stack growth
2023-05-16 19:00 [PATCH 5.10 0/3] Fix for CVE-2022-4269 Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 1/3] net/sched: act_mirred: refactor the handle of xmit Dragos-Marian Panait
@ 2023-05-16 19:00 ` Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 3/3] act_mirred: use the backlog for nested calls to mirred ingress Dragos-Marian Panait
2023-05-26 18:27 ` [PATCH 5.10 0/3] Fix for CVE-2022-4269 Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Dragos-Marian Panait @ 2023-05-16 19:00 UTC (permalink / raw)
To: stable
Cc: wenxu, Jakub Kicinski, Jamal Hadi Salim, Davide Caratti,
Marcelo Ricardo Leitner, Paolo Abeni, William Zhao, Xin Long,
David S . Miller, Eric Dumazet, Cong Wang, Jiri Pirko, Shuah Khan,
linux-kselftest, netdev
From: Davide Caratti <dcaratti@redhat.com>
[ Upstream commit 78dcdffe0418ac8f3f057f26fe71ccf4d8ed851f ]
with commit e2ca070f89ec ("net: sched: protect against stack overflow in
TC act_mirred"), act_mirred protected itself against excessive stack growth
using per_cpu counter of nested calls to tcf_mirred_act(), and capping it
to MIRRED_RECURSION_LIMIT. However, such protection does not detect
recursion/loops in case the packet is enqueued to the backlog (for example,
when the mirred target device has RPS or skb timestamping enabled). Change
the wording from "recursion" to "nesting" to make it more clear to readers.
CC: Jamal Hadi Salim <jhs@mojatatu.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>
Stable-dep-of: ca22da2fbd69 ("act_mirred: use the backlog for nested calls to mirred ingress")
Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
---
net/sched/act_mirred.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 53594b0466eb..01a44c3e8d6d 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -28,8 +28,8 @@
static LIST_HEAD(mirred_list);
static DEFINE_SPINLOCK(mirred_list_lock);
-#define MIRRED_RECURSION_LIMIT 4
-static DEFINE_PER_CPU(unsigned int, mirred_rec_level);
+#define MIRRED_NEST_LIMIT 4
+static DEFINE_PER_CPU(unsigned int, mirred_nest_level);
static bool tcf_mirred_is_act_redirect(int action)
{
@@ -225,7 +225,7 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
struct sk_buff *skb2 = skb;
bool m_mac_header_xmit;
struct net_device *dev;
- unsigned int rec_level;
+ unsigned int nest_level;
int retval, err = 0;
bool use_reinsert;
bool want_ingress;
@@ -236,11 +236,11 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
int mac_len;
bool at_nh;
- rec_level = __this_cpu_inc_return(mirred_rec_level);
- if (unlikely(rec_level > MIRRED_RECURSION_LIMIT)) {
+ nest_level = __this_cpu_inc_return(mirred_nest_level);
+ if (unlikely(nest_level > MIRRED_NEST_LIMIT)) {
net_warn_ratelimited("Packet exceeded mirred recursion limit on dev %s\n",
netdev_name(skb->dev));
- __this_cpu_dec(mirred_rec_level);
+ __this_cpu_dec(mirred_nest_level);
return TC_ACT_SHOT;
}
@@ -310,7 +310,7 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
err = tcf_mirred_forward(res->ingress, skb);
if (err)
tcf_action_inc_overlimit_qstats(&m->common);
- __this_cpu_dec(mirred_rec_level);
+ __this_cpu_dec(mirred_nest_level);
return TC_ACT_CONSUMED;
}
}
@@ -322,7 +322,7 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
if (tcf_mirred_is_act_redirect(m_eaction))
retval = TC_ACT_SHOT;
}
- __this_cpu_dec(mirred_rec_level);
+ __this_cpu_dec(mirred_nest_level);
return retval;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 5.10 3/3] act_mirred: use the backlog for nested calls to mirred ingress
2023-05-16 19:00 [PATCH 5.10 0/3] Fix for CVE-2022-4269 Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 1/3] net/sched: act_mirred: refactor the handle of xmit Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 2/3] net/sched: act_mirred: better wording on protection against excessive stack growth Dragos-Marian Panait
@ 2023-05-16 19:00 ` Dragos-Marian Panait
2023-05-26 18:27 ` [PATCH 5.10 0/3] Fix for CVE-2022-4269 Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Dragos-Marian Panait @ 2023-05-16 19:00 UTC (permalink / raw)
To: stable
Cc: wenxu, Jakub Kicinski, Jamal Hadi Salim, Davide Caratti,
Marcelo Ricardo Leitner, Paolo Abeni, William Zhao, Xin Long,
David S . Miller, Eric Dumazet, Cong Wang, Jiri Pirko, Shuah Khan,
linux-kselftest, netdev
From: Davide Caratti <dcaratti@redhat.com>
[ Upstream commit ca22da2fbd693b54dc8e3b7b54ccc9f7e9ba3640 ]
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.1663945716.git.dcaratti@redhat.com/
[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>
[DP: adjusted context for linux-5.10.y]
Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
---
net/sched/act_mirred.c | 7 +++
.../selftests/net/forwarding/tc_actions.sh | 48 ++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 01a44c3e8d6d..296af520817d 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -206,12 +206,19 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
return err;
}
+static bool is_mirred_nested(void)
+{
+ return unlikely(__this_cpu_read(mirred_nest_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 d9eca227136b..1e27031288c8 100755
--- a/tools/testing/selftests/net/forwarding/tc_actions.sh
+++ b/tools/testing/selftests/net/forwarding/tc_actions.sh
@@ -3,7 +3,7 @@
ALL_TESTS="gact_drop_and_ok_test mirred_egress_redirect_test \
mirred_egress_mirror_test matchall_mirred_egress_mirror_test \
- gact_trap_test"
+ gact_trap_test mirred_egress_to_ingress_tcp_test"
NUM_NETIFS=4
source tc_common.sh
source lib.sh
@@ -153,6 +153,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]}
--
2.40.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 5.10 0/3] Fix for CVE-2022-4269
2023-05-16 19:00 [PATCH 5.10 0/3] Fix for CVE-2022-4269 Dragos-Marian Panait
` (2 preceding siblings ...)
2023-05-16 19:00 ` [PATCH 5.10 3/3] act_mirred: use the backlog for nested calls to mirred ingress Dragos-Marian Panait
@ 2023-05-26 18:27 ` Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2023-05-26 18:27 UTC (permalink / raw)
To: Dragos-Marian Panait
Cc: stable, wenxu, Jakub Kicinski, Jamal Hadi Salim, Davide Caratti,
Marcelo Ricardo Leitner, Paolo Abeni, William Zhao, Xin Long,
David S . Miller, Eric Dumazet, Cong Wang, Jiri Pirko, Shuah Khan,
linux-kselftest, netdev
On Tue, May 16, 2023 at 10:00:37PM +0300, Dragos-Marian Panait wrote:
> The following commits are needed to fix CVE-2022-4269:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fa6d639930ee5cd3f932cc314f3407f07a06582d
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=78dcdffe0418ac8f3f057f26fe71ccf4d8ed851f
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ca22da2fbd693b54dc8e3b7b54ccc9f7e9ba3640
All now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-26 18:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-16 19:00 [PATCH 5.10 0/3] Fix for CVE-2022-4269 Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 1/3] net/sched: act_mirred: refactor the handle of xmit Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 2/3] net/sched: act_mirred: better wording on protection against excessive stack growth Dragos-Marian Panait
2023-05-16 19:00 ` [PATCH 5.10 3/3] act_mirred: use the backlog for nested calls to mirred ingress Dragos-Marian Panait
2023-05-26 18:27 ` [PATCH 5.10 0/3] Fix for CVE-2022-4269 Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).