netdev - Re: Question: Patch:("net: sched: cbq: dont intepret cls results when asked to drop") may be not bug for branch LTS 5.10

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

Date:   Tue, 17 Jan 2023 14:07:05 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     shaozhengchao <shaozhengchao@...wei.com>
Cc:     zengyhkyle@...il.com, David Miller <davem@...emloft.net>,
        Sasha Levin <sashal@...nel.org>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Davide Caratti <dcaratti@...hat.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: Question: Patch:("net: sched: cbq: dont intepret cls results when
 asked to drop") may be not bug for branch LTS 5.10

+Cc netdev

On Tue, Jan 17, 2023 at 10:06 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
>
> Trimmed Cc (+Davide).
>
> I am not sure i followed what you are saying because i dont see the
> relationship between the
> two commits. Did that patch(9410c9409d3e) cause a problem?
> How do you reproduce the issue that is caused by this patch that you are seeing?
>
> One of the challenges we have built over time is consumers of classification and
> action execution may not be fully conserving the semantics of the return code.
> The return code is a "verdict" on what happened; a safer approach is to get the
> return code to be either an error/success code. But that seems to be a
> separate issue.
>
> cheers,
> jamal
>
> On Mon, Jan 16, 2023 at 3:28 AM shaozhengchao <shaozhengchao@...wei.com> wrote:
> >
> > When I analyzed the following LTS 5.10 patch, I had a small question:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.10.y&id=b2c917e510e5ddbc7896329c87d20036c8b82952
> >
> > As described in this patch, res is obtained through the tcf_classify()
> > interface. If result is TC_ACT_SHOT, res may be an abnormal value.
> > Accessing class in res will cause abnormal access.
> >
> > For LTS version 5.10, if tcf_classify() is to return a positive value,
> > the classify hook function to the filter must be called, and the hook
> > function returns a positive number. Observe the classify function of
> > each filter. Generally, res is initialized in four scenarios.
> > 1. res is assigned a value by res in the private member of each filter.
> > Generally, kzalloc is used to assign initial values to res of various
> > filters. Therefore, class in res is initialized to 0. Then use the
> > tcf_bind_filter() interface to assign values to members in res.
> > Therefore, value of class is assigned. For example, cls_basic.
> > 2. The classify function of the filter directly assigns a value to the
> > class of res, for example, cls_cgroup.
> > 3. The filter classify function references tp and assigns a value to
> > res, for example, cls_u32.
> > 4. The change function of the filter references fh and assigns a value
> > to class in res, for example, cls_rsvp.
> >
> > This Mainline problem is caused by commit:3aa260559455 (" net/sched:
> > store the last executed chain also for clsact egress") and
> > commit:9410c9409d3e ("net: sched: Introduce ingress classification
> > function"). I don't know if my analysis is correct, please help correct,
> > thank you very much.
> >
> > Zhengchao Shao