netdev - Re: [PATCH net v2] net/sched: sch_ets: don't peek at classes beyond 'nbands'

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_iQpWFx_LXCE7YSY4NZdjkyhuhCmngXRqrn1Rze82o5Ffb9g@mail.gmail.com>
Date:   Tue, 23 Nov 2021 16:44:46 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     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>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [PATCH net v2] net/sched: sch_ets: don't peek at classes beyond 'nbands'

On Tue, Nov 23, 2021 at 5:54 AM Davide Caratti <dcaratti@...hat.com> wrote:
>
> when the number of DRR classes decreases, the round-robin active list can
> contain elements that have already been freed in ets_qdisc_change(). As a
> consequence, it's possible to see a NULL dereference crash, caused by the
> attempt to call cl->qdisc->ops->peek(cl->qdisc) when cl->qdisc is NULL:

Where exactly is it set to NULL? In line 688?

686         for (i = q->nbands; i < oldbands; i++) {
687                 qdisc_put(q->classes[i].qdisc);
688                 q->classes[i].qdisc = NULL;
689                 q->classes[i].quantum = 0;
690                 q->classes[i].deficit = 0;
691                 gnet_stats_basic_sync_init(&q->classes[i].bstats);
692                 memset(&q->classes[i].qstats, 0,
sizeof(q->classes[i].qstats));
693         }

If so, your patch is not sufficient as the NULL assignment can happen
after the check you add here?

Thanks.