Wed, May 16, 2018 at 09:47:32AM CEST, vladbu@mellanox.com wrote:

On Wed 16 May 2018 at 07:43, Jiri Pirko [off-list ref] wrote:

quoted

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

quoted

Return from action init function with reference to action taken,
even when overwriting existing action.

Action init API initializes its fourth argument (pointer to pointer to
tc action) to either existing action with same index or newly created
action. In case of existing index(and bind argument is zero), init
function returns without incrementing action reference counter. Caller
of action init then proceeds working with action without actually
holding reference to it. This means that action could be deleted
concurrently. To prevent such scenario this patch changes action init

Be imperative to the codebase in the patch description.

quoted

behavior to always take reference to action before returning
successfully.

Where's the balance? Who does the release instead? I'm probably missing
something.

I've resplit these patches for V2 to always do take/release in same
patch.

Good. Thanks.

quoted

quoted

Signed-off-by: Vlad Buslov <redacted>
---
net/sched/act_bpf.c        |  8 ++++----
net/sched/act_connmark.c   |  5 +++--
net/sched/act_csum.c       |  8 ++++----
net/sched/act_gact.c       |  5 +++--
net/sched/act_ife.c        | 12 +++++-------
net/sched/act_ipt.c        |  5 +++--
net/sched/act_mirred.c     |  5 ++---
net/sched/act_nat.c        |  5 +++--
net/sched/act_pedit.c      |  5 +++--
net/sched/act_police.c     |  8 +++-----
net/sched/act_sample.c     |  8 +++-----
net/sched/act_simple.c     |  5 +++--
net/sched/act_skbedit.c    |  5 +++--
net/sched/act_skbmod.c     |  8 +++-----
net/sched/act_tunnel_key.c |  8 +++-----
net/sched/act_vlan.c       |  8 +++-----
16 files changed, 51 insertions(+), 57 deletions(-)

diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 5d95c43..5554bf7 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c

@@ -311,9 +311,10 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,

		if (bind)
			return 0;

-		tcf_idr_release(*act, bind);
-		if (!replace)
+		if (!replace) {
+			tcf_idr_release(*act, bind);
			return -EEXIST;
+		}
	}

	is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS];

[...]