Re: [PATCH] net: sched: teql: fix use-after-free in teql_master_xmit

[Jamal Hadi Salim]

On Mon, Apr 13, 2026 at 5:44 AM Kito Xu (veritas501) <hxzene@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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
>