Skip to content
/ linux Public

Commit e9c66d3

Browse files
jhsmtgregkh
authored andcommitted
net/sched: teql: Fix double-free in teql_master_xmit
[ Upstream commit 6636046 ] Whenever a TEQL devices has a lockless Qdisc as root, qdisc_reset should be called using the seq_lock to avoid racing with the datapath. Failure to do so may cause crashes like the following: [ 238.028993][ T318] BUG: KASAN: double-free in skb_release_data (net/core/skbuff.c:1139) [ 238.029328][ T318] Free of addr ffff88810c67ec00 by task poc_teql_uaf_ke/318 [ 238.029749][ T318] [ 238.029900][ T318] CPU: 3 UID: 0 PID: 318 Comm: poc_teql_ke Not tainted 7.0.0-rc3-00149-ge5b31d988a41 #704 PREEMPT(full) [ 238.029906][ T318] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 238.029910][ T318] Call Trace: [ 238.029913][ T318] <TASK> [ 238.029916][ T318] dump_stack_lvl (lib/dump_stack.c:122) [ 238.029928][ T318] print_report (mm/kasan/report.c:379 mm/kasan/report.c:482) [ 238.029940][ T318] ? skb_release_data (net/core/skbuff.c:1139) [ 238.029944][ T318] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:221) ... [ 238.029957][ T318] ? skb_release_data (net/core/skbuff.c:1139) [ 238.029969][ T318] kasan_report_invalid_free (mm/kasan/report.c:221 mm/kasan/report.c:563) [ 238.029979][ T318] ? skb_release_data (net/core/skbuff.c:1139) [ 238.029989][ T318] check_slab_allocation (mm/kasan/common.c:231) [ 238.029995][ T318] kmem_cache_free (mm/slub.c:2637 (discriminator 1) mm/slub.c:6168 (discriminator 1) mm/slub.c:6298 (discriminator 1)) [ 238.030004][ T318] skb_release_data (net/core/skbuff.c:1139) ... [ 238.030025][ T318] sk_skb_reason_drop (net/core/skbuff.c:1256) [ 238.030032][ T318] pfifo_fast_reset (./include/linux/ptr_ring.h:171 ./include/linux/ptr_ring.h:309 ./include/linux/skb_array.h:98 net/sched/sch_generic.c:827) [ 238.030039][ T318] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:221) ... [ 238.030054][ T318] qdisc_reset (net/sched/sch_generic.c:1034) [ 238.030062][ T318] teql_destroy (./include/linux/spinlock.h:395 net/sched/sch_teql.c:157) [ 238.030071][ T318] __qdisc_destroy (./include/net/pkt_sched.h:328 net/sched/sch_generic.c:1077) [ 238.030077][ T318] qdisc_graft (net/sched/sch_api.c:1062 net/sched/sch_api.c:1053 net/sched/sch_api.c:1159) [ 238.030089][ T318] ? __pfx_qdisc_graft (net/sched/sch_api.c:1091) [ 238.030095][ T318] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:221) [ 238.030102][ T318] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:221) [ 238.030106][ T318] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:221) [ 238.030114][ T318] tc_get_qdisc (net/sched/sch_api.c:1529 net/sched/sch_api.c:1556) ... [ 238.072958][ T318] Allocated by task 303 on cpu 5 at 238.026275s: [ 238.073392][ T318] kasan_save_stack (mm/kasan/common.c:58) [ 238.073884][ T318] kasan_save_track (mm/kasan/common.c:64 (discriminator 5) mm/kasan/common.c:79 (discriminator 5)) [ 238.074230][ T318] __kasan_slab_alloc (mm/kasan/common.c:369) [ 238.074578][ T318] kmem_cache_alloc_node_noprof (./include/linux/kasan.h:253 mm/slub.c:4542 mm/slub.c:4869 mm/slub.c:4921) [ 238.076091][ T318] kmalloc_reserve (net/core/skbuff.c:616 (discriminator 107)) [ 238.076450][ T318] __alloc_skb (net/core/skbuff.c:713) [ 238.076834][ T318] alloc_skb_with_frags (./include/linux/skbuff.h:1383 net/core/skbuff.c:6763) [ 238.077178][ T318] sock_alloc_send_pskb (net/core/sock.c:2997) [ 238.077520][ T318] packet_sendmsg (net/packet/af_packet.c:2926 net/packet/af_packet.c:3019 net/packet/af_packet.c:3108) [ 238.081469][ T318] [ 238.081870][ T318] Freed by task 299 on cpu 1 at 238.028496s: [ 238.082761][ T318] kasan_save_stack (mm/kasan/common.c:58) [ 238.083481][ T318] kasan_save_track (mm/kasan/common.c:64 (discriminator 5) mm/kasan/common.c:79 (discriminator 5)) [ 238.085348][ T318] kasan_save_free_info (mm/kasan/generic.c:587 (discriminator 1)) [ 238.085900][ T318] __kasan_slab_free (mm/kasan/common.c:287) [ 238.086439][ T318] kmem_cache_free (mm/slub.c:6168 (discriminator 3) mm/slub.c:6298 (discriminator 3)) [ 238.087007][ T318] skb_release_data (net/core/skbuff.c:1139) [ 238.087491][ T318] consume_skb (net/core/skbuff.c:1451) [ 238.087757][ T318] teql_master_xmit (net/sched/sch_teql.c:358) [ 238.088116][ T318] dev_hard_start_xmit (./include/linux/netdevice.h:5324 ./include/linux/netdevice.h:5333 net/core/dev.c:3871 net/core/dev.c:3887) [ 238.088468][ T318] sch_direct_xmit (net/sched/sch_generic.c:347) [ 238.088820][ T318] __qdisc_run (net/sched/sch_generic.c:420 (discriminator 1)) [ 238.089166][ T318] __dev_queue_xmit (./include/net/sch_generic.h:229 ./include/net/pkt_sched.h:121 ./include/net/pkt_sched.h:117 net/core/dev.c:4196 net/core/dev.c:4802) Workflow to reproduce: 1. Initialize a TEQL topology (dummy0 and ifb0 as slaves, teql0 up). 2. Start multiple sender workers continuously transmitting packets through teql0 to drive teql_master_xmit(). 3. In parallel, repeatedly delete and re-add the root qdisc on dummy0 and ifb0 via RTNETLINK, forcing frequent teardown and reset activity (teql_destroy() / qdisc_reset()). 4. After running both workloads concurrently for several iterations, KASAN reports slab-use-after-free or double-free in the skb free path. Fix this by moving dev_reset_queue to sch_generic.h and calling it, instead of qdisc_reset, in teql_destroy since it handles both the lock and lockless cases correctly for root qdiscs. Fixes: 96009c7 ("sched: replace __QDISC_STATE_RUNNING bit with a spin lock") Reported-by: Xianrui Dong <keenanat2000@gmail.com> Tested-by: Xianrui Dong <keenanat2000@gmail.com> Co-developed-by: Victor Nogueira <victor@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20260315155422.147256-1-jhs@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent fd7579f commit e9c66d3

File tree

3 files changed

+30
-32
lines changed

3 files changed

+30
-32
lines changed

include/net/sch_generic.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,34 @@ void qdisc_destroy(struct Qdisc *qdisc);
696696
void qdisc_put(struct Qdisc *qdisc);
697697
void qdisc_put_unlocked(struct Qdisc *qdisc);
698698
void qdisc_tree_reduce_backlog(struct Qdisc *qdisc, int n, int len);
699+
700+
static inline void dev_reset_queue(struct net_device *dev,
701+
struct netdev_queue *dev_queue,
702+
void *_unused)
703+
{
704+
struct Qdisc *qdisc;
705+
bool nolock;
706+
707+
qdisc = rtnl_dereference(dev_queue->qdisc_sleeping);
708+
if (!qdisc)
709+
return;
710+
711+
nolock = qdisc->flags & TCQ_F_NOLOCK;
712+
713+
if (nolock)
714+
spin_lock_bh(&qdisc->seqlock);
715+
spin_lock_bh(qdisc_lock(qdisc));
716+
717+
qdisc_reset(qdisc);
718+
719+
spin_unlock_bh(qdisc_lock(qdisc));
720+
if (nolock) {
721+
clear_bit(__QDISC_STATE_MISSED, &qdisc->state);
722+
clear_bit(__QDISC_STATE_DRAINING, &qdisc->state);
723+
spin_unlock_bh(&qdisc->seqlock);
724+
}
725+
}
726+
699727
#ifdef CONFIG_NET_SCHED
700728
int qdisc_offload_dump_helper(struct Qdisc *q, enum tc_setup_type type,
701729
void *type_data);

net/sched/sch_generic.c

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,33 +1297,6 @@ static void dev_deactivate_queue(struct net_device *dev,
12971297
}
12981298
}
12991299

1300-
static void dev_reset_queue(struct net_device *dev,
1301-
struct netdev_queue *dev_queue,
1302-
void *_unused)
1303-
{
1304-
struct Qdisc *qdisc;
1305-
bool nolock;
1306-
1307-
qdisc = rtnl_dereference(dev_queue->qdisc_sleeping);
1308-
if (!qdisc)
1309-
return;
1310-
1311-
nolock = qdisc->flags & TCQ_F_NOLOCK;
1312-
1313-
if (nolock)
1314-
spin_lock_bh(&qdisc->seqlock);
1315-
spin_lock_bh(qdisc_lock(qdisc));
1316-
1317-
qdisc_reset(qdisc);
1318-
1319-
spin_unlock_bh(qdisc_lock(qdisc));
1320-
if (nolock) {
1321-
clear_bit(__QDISC_STATE_MISSED, &qdisc->state);
1322-
clear_bit(__QDISC_STATE_DRAINING, &qdisc->state);
1323-
spin_unlock_bh(&qdisc->seqlock);
1324-
}
1325-
}
1326-
13271300
static bool some_qdisc_is_busy(struct net_device *dev)
13281301
{
13291302
unsigned int i;

net/sched/sch_teql.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,12 @@ teql_destroy(struct Qdisc *sch)
146146
master->slaves = NEXT_SLAVE(q);
147147
if (q == master->slaves) {
148148
struct netdev_queue *txq;
149-
spinlock_t *root_lock;
150149

151150
txq = netdev_get_tx_queue(master->dev, 0);
152151
master->slaves = NULL;
153152

154-
root_lock = qdisc_root_sleeping_lock(rtnl_dereference(txq->qdisc));
155-
spin_lock_bh(root_lock);
156-
qdisc_reset(rtnl_dereference(txq->qdisc));
157-
spin_unlock_bh(root_lock);
153+
dev_reset_queue(master->dev,
154+
txq, NULL);
158155
}
159156
}
160157
skb_queue_purge(&dat->q);

0 commit comments

Comments
 (0)