[PATCH] net: flow_offload: clean up indirect flow block on bind failure

[PATCH] net: flow_offload: clean up indirect flow block on bind failure

[Shuangpeng Bai]

flow_indr_dev_setup_offload() records FLOW_BLOCK_BIND requests in
flow_indir_dev_list before it calls registered indirect providers. When no
registered provider binds the block, it returns -EOPNOTSUPP. The replay
entry is left behind.

One affected path is nftables netdev offload. A netdev base chain on a
device without ndo_setup_tc can take the indirect path if an indirect
provider is registered. If none of the registered providers binds the
block, the operation fails with -EOPNOTSUPP.

The failed NEWCHAIN transaction can then abort and free the nft_base_chain,
while the replay entry still points at that basechain and its flow_block
cb_list. If another provider is registered later, the stale entry is
replayed, potentially passing freed data to the provider callback or
splicing callback entries into freed memory.

Use the return value of indir_dev_add() only to track ownership of the
replay entry. Preserve the existing behavior of not propagating -EEXIST
or -ENOMEM. If this invocation inserted an entry, remove it again when
the bind fails.

Fixes: 74fc4f828769 ("net: Fix offloading indirect devices dependency on qdisc order creation")

Cc: stable@vger.kernel.org
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
 net/core/flow_offload.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 5071d7fe6ce2..5fbb1ae4679d 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -606,15 +606,20 @@ int flow_indr_dev_setup_offload(struct net_device *dev,	struct Qdisc *sch,
 				void (*cleanup)(struct flow_block_cb *block_cb))
 {
 	struct flow_indr_dev *this;
+	bool replay_added = false;
 	u32 count = 0;
 	int err;
 
 	mutex_lock(&flow_indr_block_lock);
 	if (bo) {
-		if (bo->command == FLOW_BLOCK_BIND)
-			indir_dev_add(data, dev, sch, type, cleanup, bo);
-		else if (bo->command == FLOW_BLOCK_UNBIND)
+		if (bo->command == FLOW_BLOCK_BIND) {
+			int add_err;
+
+			add_err = indir_dev_add(data, dev, sch, type, cleanup, bo);
+			replay_added = add_err == 0;
+		} else if (bo->command == FLOW_BLOCK_UNBIND) {
 			indir_dev_remove(data);
+		}
 	}
 
 	list_for_each_entry(this, &flow_block_indr_dev_list, list) {
@@ -623,9 +628,17 @@ int flow_indr_dev_setup_offload(struct net_device *dev,	struct Qdisc *sch,
 			count++;
 	}
 
+	if (bo && list_empty(&bo->cb_list)) {
+		if (replay_added)
+			indir_dev_remove(data);
+		err = -EOPNOTSUPP;
+	} else {
+		err = count;
+	}
+
 	mutex_unlock(&flow_indr_block_lock);
 
-	return (bo && list_empty(&bo->cb_list)) ? -EOPNOTSUPP : count;
+	return err;
 }
 EXPORT_SYMBOL(flow_indr_dev_setup_offload);
 
-- 
2.43.0