Skip to content

Commit 83d809a

Browse files
edumazetopsiff
authored andcommitted
net/sched: Fix mirred deadlock on device recursion
mainline inclusion from mainline-v6.9-rc5 commit 0f022d3 category: bugfix CVE: CVE-2024-27010 When the mirred action is used on a classful egress qdisc and a packet is mirrored or redirected to self we hit a qdisc lock deadlock. See trace below. [..... other info removed for brevity....] [ 82.890906] [ 82.890906] ============================================ [ 82.890906] WARNING: possible recursive locking detected [ 82.890906] 6.8.0-05205-g77fadd89fe2d-dirty deepin-community#213 Tainted: G W [ 82.890906] -------------------------------------------- [ 82.890906] ping/418 is trying to acquire lock: [ 82.890906] ffff888006994110 (&sch->q.lock){+.-.}-{3:3}, at: __dev_queue_xmit+0x1778/0x3550 [ 82.890906] [ 82.890906] but task is already holding lock: [ 82.890906] ffff888006994110 (&sch->q.lock){+.-.}-{3:3}, at: __dev_queue_xmit+0x1778/0x3550 [ 82.890906] [ 82.890906] other info that might help us debug this: [ 82.890906] Possible unsafe locking scenario: [ 82.890906] [ 82.890906] CPU0 [ 82.890906] ---- [ 82.890906] lock(&sch->q.lock); [ 82.890906] lock(&sch->q.lock); [ 82.890906] [ 82.890906] *** DEADLOCK *** [ 82.890906] [..... other info removed for brevity....] Example setup (eth0->eth0) to recreate tc qdisc add dev eth0 root handle 1: htb default 30 tc filter add dev eth0 handle 1: protocol ip prio 2 matchall \ action mirred egress redirect dev eth0 Another example(eth0->eth1->eth0) to recreate tc qdisc add dev eth0 root handle 1: htb default 30 tc filter add dev eth0 handle 1: protocol ip prio 2 matchall \ action mirred egress redirect dev eth1 tc qdisc add dev eth1 root handle 1: htb default 30 tc filter add dev eth1 handle 1: protocol ip prio 2 matchall \ action mirred egress redirect dev eth0 We fix this by adding an owner field (CPU id) to struct Qdisc set after root qdisc is entered. When the softirq enters it a second time, if the qdisc owner is the same CPU, the packet is dropped to break the loop. Reported-by: Mingshuai Ren <renmingshuai@huawei.com> Closes: https://lore.kernel.org/netdev/20240314111713.5979-1-renmingshuai@huawei.com/ Fixes: 3bcb846 ("net: get rid of spin_trylock() in net_tx_action()") Fixes: e578d9c ("net: sched: use counter to break reclassify loops") Signed-off-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Victor Nogueira <victor@mojatatu.com> Reviewed-by: Pedro Tammela <pctammela@mojatatu.com> Tested-by: Jamal Hadi Salim <jhs@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://lore.kernel.org/r/20240415210728.36949-1-victor@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 0f022d3) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
1 parent e92e482 commit 83d809a

3 files changed

Lines changed: 8 additions & 0 deletions

File tree

include/net/sch_generic.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ struct Qdisc {
117117
struct qdisc_skb_head q;
118118
struct gnet_stats_basic_sync bstats;
119119
struct gnet_stats_queue qstats;
120+
int owner;
120121
unsigned long state;
121122
unsigned long state2; /* must be written under qdisc spinlock */
122123
struct Qdisc *next_sched;

net/core/dev.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,6 +3859,10 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
38593859
return rc;
38603860
}
38613861

3862+
if (unlikely(READ_ONCE(q->owner) == smp_processor_id())) {
3863+
kfree_skb_reason(skb, SKB_DROP_REASON_TC_RECLASSIFY_LOOP);
3864+
return NET_XMIT_DROP;
3865+
}
38623866
/*
38633867
* Heuristic to force contended enqueues to serialize on a
38643868
* separate lock before trying to get qdisc main lock.
@@ -3898,7 +3902,9 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
38983902
qdisc_run_end(q);
38993903
rc = NET_XMIT_SUCCESS;
39003904
} else {
3905+
WRITE_ONCE(q->owner, smp_processor_id());
39013906
rc = dev_qdisc_enqueue(skb, q, &to_free, txq);
3907+
WRITE_ONCE(q->owner, -1);
39023908
if (qdisc_run_begin(q)) {
39033909
if (unlikely(contended)) {
39043910
spin_unlock(&q->busylock);

net/sched/sch_generic.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -983,6 +983,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
983983
sch->enqueue = ops->enqueue;
984984
sch->dequeue = ops->dequeue;
985985
sch->dev_queue = dev_queue;
986+
sch->owner = -1;
986987
netdev_hold(dev, &sch->dev_tracker, GFP_KERNEL);
987988
refcount_set(&sch->refcnt, 1);
988989

0 commit comments

Comments
 (0)