Re: [Patch net 1/2] net_sched: Flush gso_skb list too during ->change()

[Cong Wang <xiyou.wangcong@gmail.com>]

On Tue, May 06, 2025 at 02:00:36PM +0000, Savy wrote:
> On Tuesday, May 6th, 2025 at 12:15 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> 
> > 
> > the main skb queue was trimmed, potentially leaving packets in the gso_skb
> > list. This could result in NULL pointer dereference when we only check
> > sch->q.qlen against sch->limit.
> > 
> > 
> 
> Hi Cong,
> 
> With this version of the patch, the null-ptr-deref can still be triggered.
> We also need to decrement sch->q.qlen if __skb_dequeue() returns a valid skb.
> 
> We will take Codel as an example.
> 
>         while (sch->q.qlen > sch->limit) {
>                 struct sk_buff *skb = qdisc_dequeue_internal(sch, true);
>                 ...
>         }
> 
> If sch->q.qlen is 1 and there is a single packet in the gso_skb list,
> if sch->limit is dropped to 0 in codel_change, then qdisc_dequeue_internal() -> __skb_dequeue()
> will remove the skb from the gso_skb list, leaving sch->q.qlen unaltered.
> At this point, the while loop continues, as sch->q.qlen is still 1, but now both the main queue and gso_skb are empty,
> so when qdisc_dequeue_internal() is called again, it returns NULL, and the null-ptr-deref occurs.

Excellent catch!

You are right, I missed qlen--.

Also, actually I accidentally messed up the 2nd boolean parameter for
some Qdisc's.

I will update the patch and send out V2 shortly today.

Thanks!