Re: [PATCH net] net: cap skb->queue_mapping when the tx queue is picked

On Mon, Sep 21, 2026 at 10:42 AM Eric Dumazet [off-list ref] wrote:

On Mon, Sep 21, 2026 at 4:02 PM Jamal Hadi Salim [off-list ref] wrote:

quoted

skbedit can set skb->queue_mapping and __dev_queue_xmit() honors it
through the skip_txqueue flag; netdev_tx_queue_mapping() clamps the
index it uses to select the netdev_queue but leaves the out-of-range
value in skb->queue_mapping.

Every later consumer of skb_get_queue_mapping()/skb_get_tx_queue() on
that path then reads past the device's queues. Taprio's child array
q->qdiscs[] is sized to the device's queue count, so taprio_enqueue()
indexes past its allocation; qdisc_restart() likewise dereferences
dev->_tx[queue_mapping].

A local user in a network namespace can redirect a packet from a device
with more TX queues to one with fewer (mirred action) after setting a
mapping valid only on the larger device. That reaches these reads and,
under KASAN, faults with "slab-out-of-bounds in taprio_enqueue".

Store the clamped value back into skb->queue_mapping, as
netdev_core_pick_tx() already does for the mapping it picks, so the
whole egress path observes an in-range queue index.

I have looked at other alternative places to put this "fix", none
appealing: an out-of-range queue_mapping is read by every consumer
on the xmit path, not just by taprio. For example, upon testing
an approach that only bounds-checked taprio_enqueue() I observed
the fault relocated to sch_direct_xmit()/qdisc_restart() instead
(because dev->_tx[queue_mapping] is still indexed with the raw value).
Another approach was to cap it in skbedit;  cannot work: the
redirect target, whose queue count bounds the mapping, is not known
when the action runs, and act_mirred sets skb->dev afterwards.
So the decision is to cap the value where it is first trusted and
result is it fixes all downstream readers at once.

Conditions to recreate the bug: with CONFIG_NET_SCH_TAPRIO=y,
CONFIG_NET_ACT_SKBEDIT=y, CONFIG_NET_ACT_MIRRED=y and KASAN enabled,
create a 3-queue dummy qa and a 2-queue dummy qb, put a taprio root on
qb, then on qa's clsact add matchall with "action skbedit queue_mapping
2 pipe action mirred egress redirect dev qb" and send one packet out
qa. Mapping 2 is valid for qa but past qb's two-entry taprio child
array.

Reproduction: reproducer ran on a KASAN build with panic_on_warn=1:
the unfixed control faults with "BUG: KASAN: slab-out-of-bounds in
taprio_enqueue", a read 0 bytes past a 16-byte taprio_init allocation,
and panics.

I do not think this reproducer works. mirred egress redirect re-enters
__dev_queue_xmit() for qb, which does netdev_xmit_skip_txqueue(false)
before sch_handle_egress(). qb has no egress filter here, so
netdev_tx_queue_mapping() is not called for qb at all, and
netdev_core_pick_tx() rewrites skb->queue_mapping in range before
taprio_enqueue() runs.

The repro from zdi works (and produced the kasan) - but i think you
caught a bad description on how to reproduce.
It reads like it is two devices but it is infact 3:

  qa (3 queues) - clsact egress matchall:
          action skbedit queue_mapping 2 pipe
           action mirred egress redirect dev qb
  qb (2 queues, taprio root) - clsact egress matchall:
       action mirred egress mirror dev qc
  qc (1 queue) - clsact egress matchall:
       action skbedit queue_mapping 0 pipe

The KASAN trace shows exactly that stack (tcf_mirred_act ->
tcf_skbedit_act on qc, then taprio_enqueue on the outer qb xmit).

At a minimum, the commit log needs fixing to describe as above:

I suspect the real issue is that the per-cpu flag is cleared *before*
sch_handle_egress(), so a nested dev_queue_xmit() (mirred mirror, or a
drop after skbedit) can set it and have the *outer* __dev_queue_xmit()
consume it, for an skb that never went through skbedit.

That could be the root cause or part of it....

For a forwarded
packet, skb->queue_mapping is then still skb_record_rx_queue()'s
rx_queue + 1 from the ingress NIC. That reaches taprio with a garbage
index, and clamping it only hides the fact that an innocent packet got
pinned to a random TX queue.

If that is confirmed, I would rather consume the flag where it is read:

    skip_txq = netdev_xmit_txqueue_skipped();
    netdev_xmit_skip_txqueue(false);

right after sch_handle_egress(), dropping the pre-clear. Same number of
stores, and the request can no longer escape the xmit that set it.

I believe your suggestion would fix the poc repro but opens other issues.

How about: save it, clear it before sch_handle_egress(), and restore
it on every exit? Something like:

        skip_txq = netdev_xmit_txqueue_skipped();
        netdev_xmit_skip_txqueue(false);
        ...
        skb = sch_handle_egress(skb, &rc, dev);
        if (!skb) {
                netdev_xmit_skip_txqueue(skip_txq);
                goto out;
        }
        ...
        if (netdev_xmit_txqueue_skipped())
                txq = netdev_tx_queue_mapping(dev, skb);
        netdev_xmit_skip_txqueue(skip_txq);

Also, taprio_enqueue() is the only qdisc indexing a num_tx_queues sized
array with skb->queue_mapping. A bounds check there is free and worth
having regardless.

Likely missing something: how will taprio code be hit if we cover the
egress direction hole above?

cheers,
jamal