* [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit
@ 2026-04-13 9:44 Kito Xu (veritas501)
2026-04-15 15:31 ` Simon Horman
2026-04-16 15:43 ` Jamal Hadi Salim
0 siblings, 2 replies; 3+ messages in thread
From: Kito Xu (veritas501) @ 2026-04-13 9:44 UTC (permalink / raw)
To: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, netdev, linux-kernel, Kito Xu (veritas501)
teql_destroy() traverses the circular slave list to unlink a slave
qdisc. It reads master->slaves as both the starting point and the
do-while termination sentinel. However, the data-path writers
teql_dequeue() and teql_master_xmit() concurrently modify
master->slaves without holding RTNL, running in softirq or process
context respectively. If master->slaves is overwritten to an
already-visited node mid-traversal, the loop exits early without
finding the target slave. The slave is never unlinked, but its memory
is freed by call_rcu() -- leaving a dangling pointer in the circular
list. The next teql_master_xmit() traversal dereferences freed memory.
race condition:
CPU 0 (teql_destroy, RTNL held) CPU 1 (teql_dequeue, softirq)
----------------------------------- -----------------------------
prev = master->slaves; // = A
q = NEXT_SLAVE(A); // = B
B == A? No.
prev = B;
/* slave C's queue drains */
skb == NULL ->
dat->m->slaves = C; /* write! */
q = NEXT_SLAVE(B); // = C
C == A? No.
prev = C;
/* check: (prev=C) != master->slaves(C)?
FALSE -> loop exits! */
/* A never unlinked, freed by call_rcu */
CPU 0 (teql_master_xmit, later)
-----------------------------------
q = NEXT_SLAVE(C); // = A (freed!)
slave = qdisc_dev(A); // UAF!
Fix this by saving master->slaves into a local `head` variable at the
start of teql_destroy() and using it as a stable sentinel for the
entire traversal. Also annotate all data-path accesses to
master->slaves with READ_ONCE/WRITE_ONCE to prevent store-tearing and
compiler-introduced re-reads.
==================================================================
BUG: KASAN: slab-use-after-free in teql_master_xmit+0xeae/0x14a0
Read of size 8 at addr ffff888018074040 by task poc/162
CPU: 2 UID: 0 PID: 162 Comm: poc Not tainted 7.0.0-rc7-next-20260410 #10 PREEMPTLAZY
Call Trace:
<TASK>
dump_stack_lvl+0x64/0x80
print_report+0xd0/0x5e0
kasan_report+0xce/0x100
teql_master_xmit+0xeae/0x14a0
dev_hard_start_xmit+0xcd/0x5b0
sch_direct_xmit+0x12e/0xac0
__qdisc_run+0x3b1/0x1a70
__dev_queue_xmit+0x2257/0x3100
ip_finish_output2+0x615/0x19c0
ip_output+0x158/0x2b0
ip_send_skb+0x11b/0x160
udp_send_skb+0x64b/0xd80
udp_sendmsg+0x138c/0x1ec0
__sys_sendto+0x331/0x3a0
__x64_sys_sendto+0xe0/0x1c0
do_syscall_64+0x64/0x680
entry_SYSCALL_64_after_hwframe+0x76/0x7e
The buggy address belongs to the object at ffff888018074000
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 64 bytes inside of
freed 512-byte region [ffff888018074000, ffff888018074200)
==================================================================
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kito Xu (veritas501) <hxzene@gmail.com>
---
net/sched/sch_teql.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index ec4039a201a2..2e86397a5219 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -101,7 +101,7 @@ teql_dequeue(struct Qdisc *sch)
if (skb == NULL) {
struct net_device *m = qdisc_dev(q);
if (m) {
- dat->m->slaves = sch;
+ WRITE_ONCE(dat->m->slaves, sch);
netif_wake_queue(m);
}
} else {
@@ -136,19 +136,23 @@ teql_destroy(struct Qdisc *sch)
if (!master)
return;
- prev = master->slaves;
+ prev = READ_ONCE(master->slaves);
if (prev) {
+ struct Qdisc *head = prev;
+
do {
q = NEXT_SLAVE(prev);
if (q == sch) {
NEXT_SLAVE(prev) = NEXT_SLAVE(q);
- if (q == master->slaves) {
- master->slaves = NEXT_SLAVE(q);
- if (q == master->slaves) {
+ if (q == head) {
+ WRITE_ONCE(master->slaves,
+ NEXT_SLAVE(q));
+ if (q == NEXT_SLAVE(q)) {
struct netdev_queue *txq;
txq = netdev_get_tx_queue(master->dev, 0);
- master->slaves = NULL;
+ WRITE_ONCE(master->slaves,
+ NULL);
dev_reset_queue(master->dev,
txq, NULL);
@@ -158,7 +162,7 @@ teql_destroy(struct Qdisc *sch)
break;
}
- } while ((prev = q) != master->slaves);
+ } while ((prev = q) != head);
}
}
@@ -285,7 +289,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
int subq = skb_get_queue_mapping(skb);
struct sk_buff *skb_res = NULL;
- start = master->slaves;
+ start = READ_ONCE(master->slaves);
restart:
nores = 0;
@@ -317,7 +321,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
netdev_start_xmit(skb, slave, slave_txq, false) ==
NETDEV_TX_OK) {
__netif_tx_unlock(slave_txq);
- master->slaves = NEXT_SLAVE(q);
+ WRITE_ONCE(master->slaves, NEXT_SLAVE(q));
netif_wake_queue(dev);
master->tx_packets++;
master->tx_bytes += length;
@@ -329,7 +333,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
busy = 1;
break;
case 1:
- master->slaves = NEXT_SLAVE(q);
+ WRITE_ONCE(master->slaves, NEXT_SLAVE(q));
return NETDEV_TX_OK;
default:
nores = 1;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit
2026-04-13 9:44 [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit Kito Xu (veritas501)
@ 2026-04-15 15:31 ` Simon Horman
2026-04-16 15:43 ` Jamal Hadi Salim
1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2026-04-15 15:31 UTC (permalink / raw)
To: hxzene
Cc: 'Simon Horman', jhs, jiri, davem, edumazet, kuba, pabeni,
netdev, linux-kernel
From: 'Simon Horman' <horms@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net: sched: teql: fix use-after-free in teql_master_xmit
This patch attempts to fix a use-after-free in teql_master_xmit() by
saving master->slaves to a local head variable during teql_destroy(),
and by adding READ_ONCE()/WRITE_ONCE() to datapath accesses.
> diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> index ec4039a201a2c..2e86397a5219c 100644
> --- a/net/sched/sch_teql.c
> +++ b/net/sched/sch_teql.c
> @@ -101,7 +101,7 @@ teql_dequeue(struct Qdisc *sch)
> if (skb == NULL) {
> struct net_device *m = qdisc_dev(q);
> if (m) {
> - dat->m->slaves = sch;
> + WRITE_ONCE(dat->m->slaves, sch);
Can this lockless publication write a soon-to-be-freed pointer to
master->slaves?
If teql_dequeue() concurrently executes this write while teql_destroy()
is unlinking sch and scheduling it for deletion via call_rcu(), sch could
be published globally just as it is being destroyed.
When the RCU grace period ends, sch would be freed but still globally
referenced. The WRITE_ONCE() prevents store-tearing but does not prevent
this race condition.
> }
> } else {
[ ... ]
> @@ -136,19 +136,23 @@ teql_destroy(struct Qdisc *sch)
> if (!master)
> return;
>
> - prev = master->slaves;
> + prev = READ_ONCE(master->slaves);
> if (prev) {
> + struct Qdisc *head = prev;
> +
> do {
> q = NEXT_SLAVE(prev);
> if (q == sch) {
> NEXT_SLAVE(prev) = NEXT_SLAVE(q);
> - if (q == master->slaves) {
> - master->slaves = NEXT_SLAVE(q);
> - if (q == master->slaves) {
> + if (q == head) {
Could changing this condition introduce a use-after-free if master->slaves
is concurrently advanced?
If the datapath (teql_master_xmit() or teql_dequeue()) advances
master->slaves to point to sch after head was snapshot, head will not
equal sch.
When teql_destroy() unlinks sch, the q == head check would evaluate
to false, meaning master->slaves is not updated. This would leave
master->slaves pointing to the unlinked sch, which will then be freed.
> + WRITE_ONCE(master->slaves,
> + NEXT_SLAVE(q));
[ ... ]
> @@ -317,7 +321,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> netdev_start_xmit(skb, slave, slave_txq, false) ==
> NETDEV_TX_OK) {
> __netif_tx_unlock(slave_txq);
> - master->slaves = NEXT_SLAVE(q);
> + WRITE_ONCE(master->slaves, NEXT_SLAVE(q));
Similarly to teql_dequeue() above, does this risk publishing a dying qdisc
globally?
If NEXT_SLAVE(q) is read before teql_destroy() unlinks it, but the
WRITE_ONCE() executes immediately after the unlink, it appears the
unlinked qdisc would be written to the global master->slaves pointer.
[ ... ]
While looking at the concurrency issues addressed by this patch, it appears
teql_qdisc_init() might also be affected by concurrent updates to
m->slaves:
net/sched/sch_teql.c:teql_qdisc_init() {
...
q->next = NEXT_SLAVE(m->slaves);
NEXT_SLAVE(m->slaves) = sch;
...
}
Because NEXT_SLAVE(m->slaves) evaluates m->slaves twice, if m->slaves
is concurrently advanced between these two statements, could this break
the list linkage and cause lost elements or infinite loops?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit
2026-04-13 9:44 [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit Kito Xu (veritas501)
2026-04-15 15:31 ` Simon Horman
@ 2026-04-16 15:43 ` Jamal Hadi Salim
1 sibling, 0 replies; 3+ messages in thread
From: Jamal Hadi Salim @ 2026-04-16 15:43 UTC (permalink / raw)
To: Kito Xu (veritas501)
Cc: Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, linux-kernel
On Mon, Apr 13, 2026 at 5:44 AM Kito Xu (veritas501) <hxzene@gmail.com> wrote:
>
> teql_destroy() traverses the circular slave list to unlink a slave
> qdisc. It reads master->slaves as both the starting point and the
> do-while termination sentinel. However, the data-path writers
> teql_dequeue() and teql_master_xmit() concurrently modify
> master->slaves without holding RTNL, running in softirq or process
> context respectively. If master->slaves is overwritten to an
> already-visited node mid-traversal, the loop exits early without
> finding the target slave. The slave is never unlinked, but its memory
> is freed by call_rcu() -- leaving a dangling pointer in the circular
> list. The next teql_master_xmit() traversal dereferences freed memory.
>
Do you have a reproducer?
And it would be nice to get a tdc test from you.
Also, please use assisted-by: or even suggested-by if this patch was
suggested by AI.
cheers,
jamal
> race condition:
>
> CPU 0 (teql_destroy, RTNL held) CPU 1 (teql_dequeue, softirq)
> ----------------------------------- -----------------------------
> prev = master->slaves; // = A
> q = NEXT_SLAVE(A); // = B
> B == A? No.
> prev = B;
> /* slave C's queue drains */
> skb == NULL ->
> dat->m->slaves = C; /* write! */
>
> q = NEXT_SLAVE(B); // = C
> C == A? No.
> prev = C;
> /* check: (prev=C) != master->slaves(C)?
> FALSE -> loop exits! */
> /* A never unlinked, freed by call_rcu */
>
> CPU 0 (teql_master_xmit, later)
> -----------------------------------
> q = NEXT_SLAVE(C); // = A (freed!)
> slave = qdisc_dev(A); // UAF!
>
> Fix this by saving master->slaves into a local `head` variable at the
> start of teql_destroy() and using it as a stable sentinel for the
> entire traversal. Also annotate all data-path accesses to
> master->slaves with READ_ONCE/WRITE_ONCE to prevent store-tearing and
> compiler-introduced re-reads.
>
> ==================================================================
> BUG: KASAN: slab-use-after-free in teql_master_xmit+0xeae/0x14a0
> Read of size 8 at addr ffff888018074040 by task poc/162
>
> CPU: 2 UID: 0 PID: 162 Comm: poc Not tainted 7.0.0-rc7-next-20260410 #10 PREEMPTLAZY
> Call Trace:
> <TASK>
> dump_stack_lvl+0x64/0x80
> print_report+0xd0/0x5e0
> kasan_report+0xce/0x100
> teql_master_xmit+0xeae/0x14a0
> dev_hard_start_xmit+0xcd/0x5b0
> sch_direct_xmit+0x12e/0xac0
> __qdisc_run+0x3b1/0x1a70
> __dev_queue_xmit+0x2257/0x3100
> ip_finish_output2+0x615/0x19c0
> ip_output+0x158/0x2b0
> ip_send_skb+0x11b/0x160
> udp_send_skb+0x64b/0xd80
> udp_sendmsg+0x138c/0x1ec0
> __sys_sendto+0x331/0x3a0
> __x64_sys_sendto+0xe0/0x1c0
> do_syscall_64+0x64/0x680
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> The buggy address belongs to the object at ffff888018074000
> which belongs to the cache kmalloc-512 of size 512
> The buggy address is located 64 bytes inside of
> freed 512-byte region [ffff888018074000, ffff888018074200)
> ==================================================================
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kito Xu (veritas501) <hxzene@gmail.com>
> ---
> net/sched/sch_teql.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> index ec4039a201a2..2e86397a5219 100644
> --- a/net/sched/sch_teql.c
> +++ b/net/sched/sch_teql.c
> @@ -101,7 +101,7 @@ teql_dequeue(struct Qdisc *sch)
> if (skb == NULL) {
> struct net_device *m = qdisc_dev(q);
> if (m) {
> - dat->m->slaves = sch;
> + WRITE_ONCE(dat->m->slaves, sch);
> netif_wake_queue(m);
> }
> } else {
> @@ -136,19 +136,23 @@ teql_destroy(struct Qdisc *sch)
> if (!master)
> return;
>
> - prev = master->slaves;
> + prev = READ_ONCE(master->slaves);
> if (prev) {
> + struct Qdisc *head = prev;
> +
> do {
> q = NEXT_SLAVE(prev);
> if (q == sch) {
> NEXT_SLAVE(prev) = NEXT_SLAVE(q);
> - if (q == master->slaves) {
> - master->slaves = NEXT_SLAVE(q);
> - if (q == master->slaves) {
> + if (q == head) {
> + WRITE_ONCE(master->slaves,
> + NEXT_SLAVE(q));
> + if (q == NEXT_SLAVE(q)) {
> struct netdev_queue *txq;
>
> txq = netdev_get_tx_queue(master->dev, 0);
> - master->slaves = NULL;
> + WRITE_ONCE(master->slaves,
> + NULL);
>
> dev_reset_queue(master->dev,
> txq, NULL);
> @@ -158,7 +162,7 @@ teql_destroy(struct Qdisc *sch)
> break;
> }
>
> - } while ((prev = q) != master->slaves);
> + } while ((prev = q) != head);
> }
> }
>
> @@ -285,7 +289,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> int subq = skb_get_queue_mapping(skb);
> struct sk_buff *skb_res = NULL;
>
> - start = master->slaves;
> + start = READ_ONCE(master->slaves);
>
> restart:
> nores = 0;
> @@ -317,7 +321,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> netdev_start_xmit(skb, slave, slave_txq, false) ==
> NETDEV_TX_OK) {
> __netif_tx_unlock(slave_txq);
> - master->slaves = NEXT_SLAVE(q);
> + WRITE_ONCE(master->slaves, NEXT_SLAVE(q));
> netif_wake_queue(dev);
> master->tx_packets++;
> master->tx_bytes += length;
> @@ -329,7 +333,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> busy = 1;
> break;
> case 1:
> - master->slaves = NEXT_SLAVE(q);
> + WRITE_ONCE(master->slaves, NEXT_SLAVE(q));
> return NETDEV_TX_OK;
> default:
> nores = 1;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-16 15:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-13 9:44 [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit Kito Xu (veritas501)
2026-04-15 15:31 ` Simon Horman
2026-04-16 15:43 ` Jamal Hadi Salim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox