[PATCH net-next v2] net/sched: cls_fw: allow filter replace without re-specifying mask

[Wei Tao]

The fw classifier keeps the fwmark mask in the fw_head shared by all
filters of a tcf_proto instance, so the mask is a property of the head
and not of the individual filter: it is set when the head is created
and cannot be changed by a filter that is being replaced.  Nevertheless
fw_set_parms() rejects such a replace with -EINVAL whenever the head
mask is not the default 0xFFFFFFFF and the mask is not repeated on the
command line:

  # tc qdisc add dev lo clsact
  # tc filter add dev lo ingress prio 1 handle 1/0xffff fw classid 1:10
  # tc filter replace dev lo ingress prio 1 handle 1 fw classid 1:20
  RTNETLINK answers: Invalid argument

The same happens for tc filter change, which calls fw_set_parms() on the
existing filter as well.

The check only makes sense for a newly created filter, where the mask
has to be spelled out so that a filter cannot silently match on a
non-default mask inherited from the existing head.  A filter that is
being replaced already shares the head mask and inherits it unchanged,
so the check can be skipped for it.

Tell the two cases apart with a new "new_filter" argument to
fw_set_parms(), which fw_change() sets only when it creates a filter,
and report the error through extack instead of failing silently.

When reviewing commit b72e025ac556 ("net/sched: cls_route: reject
change with no routing attribute") and its parent commit 41e85e54e564
("net/sched: cls_route: Fix in-place replace"), I noticed that the
series separates the create and replace paths in cls_route: a new
filter must use a handle that matches the value built from its
attributes, while a replace is not allowed to change the handle without
specifying routing attributes. Then I analyzed the cls_fw code with
an LLM.

Tested on aarch64:

  # tc qdisc add dev lo clsact
  # tc filter add dev lo ingress prio 1 handle 1/0xffff fw classid 1:10
  # tc filter replace dev lo ingress prio 1 handle 1 fw classid 1:20
  # tc filter show dev lo ingress | cut -d' ' -f9-
  handle 0x1/0xffff classid 1:20

  # tc filter change dev lo ingress prio 1 handle 1 fw classid 1:30
  # tc filter show dev lo ingress | cut -d' ' -f9-
  handle 0x1/0xffff classid 1:30

  # tc filter add dev lo ingress prio 1 handle 2 fw classid 1:40
  Error: Mask must be provided for new filter.

Signed-off-by: Wei Tao <taowei@xxxxxxxxxx>
---
v2: https://lore.kernel.org/netdev/20261014024548.37874-1-taowei@xxxxxxxxxx/
  - drop the Fixes tag: this is not a regression fix
  - describe how the issue was found and add the tested commands
 net/sched/cls_fw.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 0f731595f839..3ca0598560c4 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -204,7 +204,8 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = {
 
 static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 			struct fw_filter *f, struct nlattr **tb,
-			struct nlattr **tca, unsigned long base, u32 flags,
+			struct nlattr **tca, unsigned long base,
+			bool new_filter, u32 flags,
 			struct netlink_ext_ack *extack)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
@@ -229,8 +230,10 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 		mask = nla_get_u32(tb[TCA_FW_MASK]);
 		if (mask != head->mask)
 			return err;
-	} else if (head->mask != 0xFFFFFFFF)
+	} else if (head->mask != 0xFFFFFFFF && new_filter) {
+		NL_SET_ERR_MSG(extack, "Mask must be provided for new filter");
 		return err;
+	}
 
 	if (tb[TCA_FW_CLASSID]) {
 		f->res.classid = nla_get_u32(tb[TCA_FW_CLASSID]);
@@ -291,7 +294,8 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 			return err;
 		}
 
-		err = fw_set_parms(net, tp, fnew, tb, tca, base, flags, extack);
+		err = fw_set_parms(net, tp, fnew, tb, tca, base, false,
+				   flags, extack);
 		if (err < 0) {
 			tcf_exts_destroy(&fnew->exts);
 			kfree(fnew);
@@ -340,7 +344,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 	f->id = handle;
 	f->tp = tp;
 
-	err = fw_set_parms(net, tp, f, tb, tca, base, flags, extack);
+	err = fw_set_parms(net, tp, f, tb, tca, base, true, flags, extack);
 	if (err < 0)
 		goto errout;
 

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.25.1