Re: [PATCH net] ipv6: ioam: fix potential NULL dereferences in __ioam6_fill_trace_data()

Justin Iurman
2026-04-04T10:13:10Z

On 4/3/26 23:50, Jakub Kicinski wrote:
> On Fri, 3 Apr 2026 14:47:32 -0700 Eric Dumazet wrote:
>>> If the ingress device has more RX queues than the egress device (dev) has
>>> TX queues, skb_get_queue_mapping(skb) will exceed dev->num_tx_queues.
>>>
>>> Since skb_get_tx_queue() does not clamp the index, will it return an
>>> out-of-bounds pointer into the egress device's dev->_tx array, causing
>>> dereferencing queue->qdisc to read invalid memory?
>>
>> This seems a different bug ?
>>
>> I mean, do we need to fix all bug in a single patch ?
>
> no no, sorry, i'm just sending this out for Justin or you as a separate
> thing.

Thanks Jakub, I don't know how I missed that. It's not just about a
possible OOB: it's actually incorrect for IOAM to do that because we're
relying on a lucky assumption at best (i.e., if queue_mapping on RX ==
queue_mapping on TX). This unfortunately rules out the possibility to
have an accurate per queue visibility (which I originally provided for
flexibility). I've had a local patch for a long time to aggregate the
queue depth (i.e., sum of all TX queues). It is more expensive(***), but
it would kill two birds with one stone and solve both problems
simultaneously.

Jakub, Eric, please let me know if the following plan works for both of you:
- (net) fix the OOB reported by Jakub (and, while at it, add missing
locks around qdisc_qstats_qlen_backlog() -> also included in my local patch)
- (net-next) after the merge window, add the new feature

After that, the per queue visibility would be considered
legacy/deprecated and not the default behavior. Documentation would be
updated accordingly to explain why per queue visibility cannot be trusted.

(***) the advise to operators in such a case is (obviously) to reduce
the IOAM insertion (with the frequency parameter), especially at line rate.