[PATCH net v2] net/sched: cls_api: unlink indr cb on a failed bind

[PATCH net v2] net/sched: cls_api: unlink indr cb on a failed bind

[Qihang]

When a device is bound to a shared block through the indirect flow-block
path, flow_indr_block_cb_alloc() links the driver callback on
flow_block_indr_list.  If tcf_block_bind() then fails, its error path
removes the callback from driver_list and frees it, but leaves it linked
on flow_block_indr_list, so the next flow_indr_dev_unregister() of that
driver walks the freed callback.

Remove the callback from flow_block_indr_list before freeing it.  That
list is protected by flow_indr_block_lock, so do the unlink under the
lock through a new flow_indr_block_cb_unlink() helper.

commit da94a7781fc3 ("net/sched: cls_api: remove block_cb from driver_list
before freeing") fixed this same error path for driver_list; indr.list is
the link that change still misses.

flow_block_cb_alloc() initialises indr.list, so the helper is a no-op for
the direct (non-indirect) callbacks that are never on the list.

Fixes: 1fac52da5942c ("net: flow_offload: consolidate indirect flow_block infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
v2:
 - unlink the callback under flow_indr_block_lock with a new
   flow_indr_block_cb_unlink() helper; v1 called list_del_init() without
   holding the lock, which races a concurrent flow_indr_dev_unregister()
   walking flow_block_indr_list.
 - initialise indr.list in flow_block_cb_alloc() so the helper is a no-op
   for direct (non-indirect) callbacks.

v1: https://lore.kernel.org/netdev/20261013162949.25622-1-q.h.hack.winter@gmail.com/

 include/net/flow_offload.h | 1 +
 net/core/flow_offload.c    | 9 +++++++++
 net/sched/cls_api.c        | 1 +
 3 files changed, 11 insertions(+)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 70a02ee14308..b8ca8cc0a241 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -640,6 +640,7 @@ struct flow_block_cb *flow_indr_block_cb_alloc(flow_setup_cb_t *cb,
 					       void *indr_cb_priv,
 					       void (*cleanup)(struct flow_block_cb *block_cb));
 void flow_block_cb_free(struct flow_block_cb *block_cb);
+void flow_indr_block_cb_unlink(struct flow_block_cb *block_cb);
 
 struct flow_block_cb *flow_block_cb_lookup(struct flow_block *block,
 					   flow_setup_cb_t *cb, void *cb_ident);
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 5071d7fe6ce2..62bc2eb6e0da 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -266,6 +266,8 @@ struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
 	if (!block_cb)
 		return ERR_PTR(-ENOMEM);
 
+	INIT_LIST_HEAD(&block_cb->indr.list);
+
 	block_cb->cb = cb;
 	block_cb->cb_ident = cb_ident;
 	block_cb->cb_priv = cb_priv;
@@ -525,6 +527,13 @@ static void flow_block_indr_init(struct flow_block_cb *flow_block,
 	flow_block->indr.cleanup = cleanup;
 }
 
+void flow_indr_block_cb_unlink(struct flow_block_cb *block_cb)
+{
+	mutex_lock(&flow_indr_block_lock);
+	list_del_init(&block_cb->indr.list);
+	mutex_unlock(&flow_indr_block_lock);
+}
+
 struct flow_block_cb *flow_indr_block_cb_alloc(flow_setup_cb_t *cb,
 					       void *cb_ident, void *cb_priv,
 					       void (*release)(void *cb_priv),
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index a9f54988561f..9e4ea3715419 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1665,6 +1665,7 @@ static int tcf_block_bind(struct tcf_block *block,
 err_unroll:
 	list_for_each_entry_safe(block_cb, next, &bo->cb_list, list) {
 		list_del(&block_cb->driver_list);
+		flow_indr_block_cb_unlink(block_cb);
 		if (i-- > 0) {
 			list_del(&block_cb->list);
 			tcf_block_playback_offloads(block, block_cb->cb,
-- 
2.54.0 (Apple Git-157)

Re: [PATCH net v2] net/sched: cls_api: unlink indr cb on a failed bind

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.