Tue, May 15, 2018 at 01:32:51PM CEST, vladbu@mellanox.com wrote:

On Tue 15 May 2018 at 11:24, Jiri Pirko [off-list ref] wrote:

quoted

Mon, May 14, 2018 at 04:27:08PM CEST, vladbu@mellanox.com wrote:

quoted

Change action API to assume that action init function always takes
reference to action, even when overwriting existing action. This is
necessary because action API continues to use action pointer after init
function is done. At this point action becomes accessible for concurrent
modifications so user must always hold reference to it.

Implement helper put list function to atomically release list of actions
after action API init code is done using them.

Signed-off-by: Vlad Buslov <redacted>
---
net/sched/act_api.c | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)

[...]

quoted

@@ -1196,8 +1190,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,

		return ret;
	}
err:
-	if (event != RTM_GETACTION)

Howcome you do this for RTM_GETACTION now too? Where is the related
"get"?

In patch 5. There is always a possibility of concurrent delete without
rtnl lock so all usages of action pointers were converted to hold
reference to action.

So that means that if you run kernel in between, with patch 5 but
without patch 7 and you do RTM_GETACTION, you leak a reference, right?

quoted

quoted

-		tcf_action_destroy(&actions, 0);
+	tcf_action_put_lst(&actions);
	return ret;
}

[...]