All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: sched: fix infinite loop when trying to create tcf rule
@ 2021-10-13 21:09 Paul Menzel
  2021-10-14 14:19 ` Jakub Kicinski
  0 siblings, 1 reply; 2+ messages in thread
From: Paul Menzel @ 2021-10-13 21:09 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller, Jakub Kicinski
  Cc: Serhiy Boiko, Paul Menzel, netdev, linux-kernel

From: Serhiy Boiko <serhiy.boiko@plvision.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@plvision.eu>
[Upstreamed from https://github.com/dentproject/dentOS/commit/3480aceaa5244a11edfe1fda90afd92b0199ef23]
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 net/sched/cls_api.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2ef8f5a6205a..405f955bef1e 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -563,8 +563,6 @@ static void __tcf_chain_put(struct tcf_chain *chain, bool by_act,
 	if (refcnt - chain->action_refcnt == 0 && !by_act) {
 		tc_chain_notify_delete(tmplt_ops, tmplt_priv, chain->index,
 				       block, NULL, 0, 0, false);
-		/* Last reference to chain, no need to lock. */
-		chain->flushing = false;
 	}
 
 	if (refcnt == 0)
@@ -615,6 +613,9 @@ static void tcf_chain_flush(struct tcf_chain *chain, bool rtnl_held)
 		tcf_proto_put(tp, rtnl_held, NULL);
 		tp = tp_next;
 	}
+
+	/* Last reference to chain, no need to lock. */
+	chain->flushing = false;
 }
 
 static int tcf_block_setup(struct tcf_block *block,
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: sched: fix infinite loop when trying to create tcf rule
  2021-10-13 21:09 [PATCH] net: sched: fix infinite loop when trying to create tcf rule Paul Menzel
@ 2021-10-14 14:19 ` Jakub Kicinski
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2021-10-14 14:19 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
	Serhiy Boiko, netdev, Volodymyr Mytnyk, Vlad Buslov

On Wed, 13 Oct 2021 23:09:59 +0200 Paul Menzel wrote:
> From: Serhiy Boiko <serhiy.boiko@plvision.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@plvision.eu>
> [Upstreamed from https://github.com/dentproject/dentOS/commit/3480aceaa5244a11edfe1fda90afd92b0199ef23]
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>

This had already been submitted:

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-10-14 14:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 21:09 [PATCH] net: sched: fix infinite loop when trying to create tcf rule Paul Menzel
2021-10-14 14:19 ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.