netdev - Re: [Patch net 1/2] net_sched: Flush gso

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aBpzr1ey7MmHObLR@pop-os.localdomain>
Date: Tue, 6 May 2025 13:40:15 -0700
From: Cong Wang <xiyou.wangcong@...il.com>
To: Savy <savy@...t3mfailure.io>
Cc: netdev@...r.kernel.org, jiri@...nulli.us, jhs@...atatu.com,
	willsroot@...tonmail.com
Subject: Re: [Patch net 1/2] net_sched: Flush gso_skb list too during
 ->change()

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@...il.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!