netdev - Re: [PATCH] net: sched: fix infinite loop when trying to create tcf rule

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: <20211014071909.63338451@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 14 Oct 2021 07:19:09 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Paul Menzel <pmenzel@...gen.mpg.de>
Cc:     Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        Serhiy Boiko <serhiy.boiko@...ision.eu>,
        netdev@...r.kernel.org,
        Volodymyr Mytnyk <volodymyr.mytnyk@...ision.eu>,
        Vlad Buslov <vladbu@...lanox.com>
Subject: Re: [PATCH] net: sched: fix infinite loop when trying to create tcf
 rule

On Wed, 13 Oct 2021 23:09:59 +0200 Paul Menzel wrote:
> From: Serhiy Boiko <serhiy.boiko@...ision.eu>
> 
> After running a specific set of commands tc will become unresponsive:
> 
>     $ ip link add dev DEV type veth
>     $ tc qdisc add dev DEV clsact
>     $ tc chain add dev DEV chain 0 ingress
>     $ tc filter del dev DEV ingress
>     $ tc filter add dev DEV ingress flower action pass
> 
> When executing chain flush the chain->flushing field is set to true, which
> prevents insertion of new classifier instances.  It is unset in one place
> under two conditions: `refcnt - chain->action_refcnt == 0` and `!by_act`.
> Ignoring the by_act and action_refcnt arguments the `flushing procedure`
> will be over when refcnt is 0.
> 
> But if the chain is explicitly created (e.g. `tc chain add .. chain 0 ..`)
> refcnt is set to 1 without any classifier instances. Thus the condition is
> never met and the chain->flushing field is never cleared.  And because the
> default chain is `flushing` new classifiers cannot be added. tc_new_tfilter
> is stuck in a loop trying to find a chain where chain->flushing is false.
> 
> Moving `chain->flushing = false` from __tcf_chain_put to the end of
> tcf_chain_flush will avoid the condition and the field will always be reset
> after the flush procedure.
> 
> Signed-off-by: Serhiy Boiko <serhiy.boiko@...ision.eu>
> [Upstreamed from https://github.com/dentproject/dentOS/commit/3480aceaa5244a11edfe1fda90afd92b0199ef23]
> Signed-off-by: Paul Menzel <pmenzel@...gen.mpg.de>

This had already been submitted:

https://lore.kernel.org/all/1633848948-11315-1-git-send-email-volodymyr.mytnyk@plvision.eu/