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

[PATCH net] 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.

Fixes: b4e9b520ca5d ("[NET_SCHED]: Add mask support to fwmark classifier")
Signed-off-by: Wei Tao <taowei@kylinos.cn>
---
 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 a462b262719c..f1b648a11e7c 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;
 
-- 
2.33.0

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

[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.

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.