Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
To:     netdev@vger.kernel.org
Cc:     Volodymyr Mytnyk <vmytnyk@marvell.com>,
        Serhiy Boiko <serhiy.boiko@plvision.eu>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Vlad Buslov <vladbu@mellanox.com>, linux-kernel@vger.kernel.org
Subject: [PATCH net] sched: fix infinite loop when creating tc filter
Date:   Sun, 10 Oct 2021 09:55:47 +0300
Message-Id: <1633848948-11315-1-git-send-email-volodymyr.mytnyk@plvision.eu>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?dbODmvkmoC6YLHu+a3CPyaKvlpwpr4GjaA1rhx7T5gqBHjWhaiiWzPyh54Ix?=
 =?us-ascii?Q?57ggIgSXKN/HGZJ8Gl2HOc2qAAWu5lqNXYgYjX3Nxtrv+87EFdSwfRc20TxI?=
 =?us-ascii?Q?8nRhucC6+F0q6FBr995atJ1rfuzvF2YkoRpw95JpLeNqFE1XHjXVLWOA2HP2?=
 =?us-ascii?Q?vpAnnaJhf23wS5BRHMUp1ti32bZMfKAm/jYMIn91DCmUdhIsdiM+CfbGZ1BF?=
 =?us-ascii?Q?nr6x4apIYHMwM7ET5JP0odE2vJ1FBNw879VRUtj26+jG2HmcV+2lCl9yVqH5?=
 =?us-ascii?Q?jLkWMPYd3TUTRXmUb6kIZyrP6sUw2i/igDHtpYGCHJ6TY6EcmccJI3awDcQB?=
 =?us-ascii?Q?S6i6no/mzeqx7MOWM6UFGPlc2d5jr5sqO+bNMbcSmLG3CIK5ppz92sjZVrUs?=
 =?us-ascii?Q?YTNSKaZpCNqCGAiQ6HURKrKExiGs2QVz+ZAdAqioIFXMmVGiScrGJM9z44Uq?=
 =?us-ascii?Q?a+VKZfc7eSh9UZsEjJ3FnJ/8uOKibdkZrJ4rEaq/vYt+1Pxa9CCHCOUOTH3D?=
 =?us-ascii?Q?RMwcLLp5ovoRkiDowhJpANvjEtkigXVxURD411+6yKxhhNC8Y4qKlwCIMuQm?=
 =?us-ascii?Q?QxTwlTwq3MccCQUYHqkMTDwY3fmZP8Alwp6SoQDlW9SxmPGemMX8ST1HBWTW?=
 =?us-ascii?Q?aNWI3tHFLEaH1wJVYydv9SaBIAJL6IjD8SZ6l2FciYLCVJgApsZb+iO3w93R?=
 =?us-ascii?Q?GuFe5D08+dyB1ivYttstiGzTF7HqVgQ6+YjLtxS+ANf2BbURWKR9X0kHlEHJ?=
 =?us-ascii?Q?4DkCgBg1OCC1zEJqfBhjr1/ZTVPPWfxXoWokJocfIwkwiRq/4PLljbCeqHb9?=
 =?us-ascii?Q?tdhIC1RV7Ge8RsmOeA1hplrty/EhXATBij7ecqlSDQqdtBxsc5lZDoojWLM+?=
 =?us-ascii?Q?sCkiwT2MclkNVX7u9fWcJbtQZi4P1ljxehYJCc5sto4FgYv71u5F49UR3ybk?=
 =?us-ascii?Q?Q2F7Z+EJrDT9RWTUgH0uxYWTLqNrIiEn7tBPzd3/NWlK4HWIbhd4lmwxTX2E?=
 =?us-ascii?Q?lHCP229+kzbekkCV+wrYh84WguwzCvaNI5roT7VwNdPSV+3DgX+WV1ZZDhb0?=
 =?us-ascii?Q?hm314O+KKR3LMfIhpTjctuL8/WYe+5YJqtNomuhH/s96inhe+h2c9A9m7LyU?=
 =?us-ascii?Q?BPfQvxBZIKy4yqfINpTJeQO7bmgB0LJbLvN1Uz6uBntzce+2wPcmVm+1XoRZ?=
 =?us-ascii?Q?G1qCa8JnXsNHEn6FxwkxVxNXyJYAGXfOkHzHWmO/Dj68p2Pe6AOpZxCY/0Bv?=
 =?us-ascii?Q?jcwNSJIg2Ot0WfzXb/33paeuID9sczhgHcN5QUHHGIHqo3X4yYKvkpcAHlhn?=
 =?us-ascii?Q?pkGfezSJ2WRG6wA2jNnQ19Ij?=
X-OriginatorOrg: plvision.eu
X-MS-Exchange-CrossTenant-Network-Message-Id: f3d7062b-3a82-40fd-bb7b-08d98bbb0bd9
X-MS-Exchange-CrossTenant-AuthSource: VI1P190MB0734.EURP190.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Oct 2021 06:56:12.5740
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 03707b74-30f3-46b6-a0e0-ff0a7438c9c4
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 04OmkH6shAOEGBK3wo/ENOf2ToqfNOQYYycY25KUQUp5912tsieZ+TQ+73k9it1Zcpd2DoMHu0aFxHLlvHpaD3TuNCOs/A7Xvftx/m5ntG8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1P190MB0223
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Volodymyr Mytnyk <vmytnyk@marvell.com>

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.

By 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.

Fixes: 91052fa1c657 ("net: sched: protect chain->explicitly_created with block->lock")

Co-developed-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
---
 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 d73b5c5514a9..327594cce554 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.7.4