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] [thread-next>] [day] [month] [year] [list]

Message-ID: <krDuBwNbhtDxUlG2tgiXBwSA9KUwph1GfKqwvjBxYDSJv6nVQ98S_inVmQxaRBsndKdgg-rh_vN0xouX4zraF6V3UyQHpWNJUv-rvd-Cwfg=@syst3mfailure.io>
Date: Tue, 06 May 2025 14:00:36 +0000
From: Savy <savy@...t3mfailure.io>
To: Cong Wang <xiyou.wangcong@...il.com>
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 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.

Something like this can fix the issue (tested with Codel):

static inline struct sk_buff *qdisc_dequeue_internal(struct Qdisc *sch, bool direct)
{
        struct sk_buff *skb;

        skb = __skb_dequeue(&sch->gso_skb);
        if (skb) {
                sch->q.qlen--;
                return skb;
        }

        if (direct) {
                skb = __qdisc_dequeue_head(&sch->q);
        } else {
                skb = sch->dequeue(sch);
        }
        return skb;
}

Regards,
Savy
Will