netdev - Re: [PATH net] net/sched: ets: fix crash when flipping from 'strict' to 'quantum'

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: <CAM_iQpUryQ8Q9cd9Oiv=hxAgpqfCz=j4E=c=hskbPE2+VB-ZvQ@mail.gmail.com>
Date:   Mon, 30 Aug 2021 17:43:09 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     Hangbin Liu <liuhangbin@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Petr Machata <petrm@...lanox.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATH net] net/sched: ets: fix crash when flipping from 'strict'
 to 'quantum'

On Tue, Aug 24, 2021 at 3:34 PM Davide Caratti <dcaratti@...hat.com> wrote:
> When the change() function decreases the value of 'nstrict', we must take
> into account that packets might be already enqueued on a class that flips
> from 'strict' to 'quantum': otherwise that class will not be added to the
> bandwidth-sharing list. Then, a call to ets_qdisc_reset() will attempt to
> do list_del(&alist) with 'alist' filled with zero, hence the NULL pointer
> dereference.

I am confused about how we end up having NULL in list head.

>From your changelog, you imply it happens when we change an existing
Qdisc, but I don't see any place that could set this list head to NULL.
list_del() clearly doesn't set NULL.

But if it is a new Qdisc, Qdisc is allocated with zero's hence having NULL
as list head. However, in this case, q->nstrict should be 0 before the loop,
so I don't think your code helps at all as the for loop can't even be entered?

Thanks.