public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2 1/1] net/sched: cls_flower: avoid stale mask references after delete
@ 2026-04-21 16:03 Ren Wei
  2026-04-23 18:51 ` Jakub Kicinski
  0 siblings, 1 reply; 2+ messages in thread
From: Ren Wei @ 2026-04-21 16:03 UTC (permalink / raw)
  To: netdev
  Cc: jhs, jiri, davem, edumazet, kuba, pabeni, horms, sbrivio, vladbu,
	yuantan098, yifanwucs, tomapufckgml, bird, kanolyc, z1652074432,
	n05ec

From: Yuhang Zheng <z1652074432@gmail.com>

cls_flower keeps filter and mask state separately. After a filter is
removed or replaced, some paths can still need the mask data associated
with that filter.

Cache the mask key and dissector in struct cls_fl_filter when the mask
is assigned, and use the cached copies in dump and offload paths. This
avoids depending on the external mask object's lifetime after delete or
replace.

Fixes: 92149190067d ("net: sched: flower: set unlocked flag for flower proto ops")
Cc: stable@kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Yucheng Lu <kanolyc@gmail.com>
Signed-off-by: Yuhang Zheng <z1652074432@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---

changes in v2:
- target the net tree instead of nf
- fix the Fixes tag to the first triggerable introduction
- correct the Yuan Tan Reported-by address and add the forwarder sign-off
- v1 Link: https://lore.kernel.org/all/0fdcae6ac3e07afbbd43958f6b42e2ed6281e3d2.1773559972.git.z1652074432@gmail.com

 net/sched/cls_flower.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 099ff6a3e1f5..c1f10b4ec748 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -124,8 +124,10 @@ struct cls_fl_head {
 
 struct cls_fl_filter {
 	struct fl_flow_mask *mask;
+	struct flow_dissector mask_dissector;
 	struct rhash_head ht_node;
 	struct fl_flow_key mkey;
+	struct fl_flow_key mask_key;
 	struct tcf_exts exts;
 	struct tcf_result res;
 	struct fl_flow_key key;
@@ -445,6 +447,12 @@ static void fl_destroy_filter_work(struct work_struct *work)
 	__fl_destroy_filter(f);
 }
 
+static void fl_filter_copy_mask(struct cls_fl_filter *f)
+{
+	f->mask_key = f->mask->key;
+	f->mask_dissector = f->mask->dissector;
+}
+
 static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f,
 				 bool rtnl_held, struct netlink_ext_ack *extack)
 {
@@ -476,8 +484,8 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, extack);
 	cls_flower.command = FLOW_CLS_REPLACE;
 	cls_flower.cookie = (unsigned long) f;
-	cls_flower.rule->match.dissector = &f->mask->dissector;
-	cls_flower.rule->match.mask = &f->mask->key;
+	cls_flower.rule->match.dissector = &f->mask_dissector;
+	cls_flower.rule->match.mask = &f->mask_key;
 	cls_flower.rule->match.key = &f->mkey;
 	cls_flower.classid = f->res.classid;
 
@@ -2489,6 +2497,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 	err = fl_check_assign_mask(head, fnew, fold, mask);
 	if (err)
 		goto unbind_filter;
+	fl_filter_copy_mask(fnew);
 
 	err = fl_ht_insert_unique(fnew, fold, &in_ht);
 	if (err)
@@ -2705,8 +2714,8 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb,
 		cls_flower.command = add ?
 			FLOW_CLS_REPLACE : FLOW_CLS_DESTROY;
 		cls_flower.cookie = (unsigned long)f;
-		cls_flower.rule->match.dissector = &f->mask->dissector;
-		cls_flower.rule->match.mask = &f->mask->key;
+		cls_flower.rule->match.dissector = &f->mask_dissector;
+		cls_flower.rule->match.mask = &f->mask_key;
 		cls_flower.rule->match.key = &f->mkey;
 
 		err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts,
@@ -3709,7 +3718,7 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 		goto nla_put_failure_locked;
 
 	key = &f->key;
-	mask = &f->mask->key;
+	mask = &f->mask_key;
 	skip_hw = tc_skip_hw(f->flags);
 
 	if (fl_dump_key(skb, net, key, mask))
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v2 1/1] net/sched: cls_flower: avoid stale mask references after delete
  2026-04-21 16:03 [PATCH net v2 1/1] net/sched: cls_flower: avoid stale mask references after delete Ren Wei
@ 2026-04-23 18:51 ` Jakub Kicinski
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2026-04-23 18:51 UTC (permalink / raw)
  To: jiri
  Cc: Ren Wei, netdev, jhs, davem, edumazet, pabeni, horms, sbrivio,
	vladbu, yuantan098, yifanwucs, tomapufckgml, bird, kanolyc,
	z1652074432

On Wed, 22 Apr 2026 00:03:02 +0800 Ren Wei wrote:
> From: Yuhang Zheng <z1652074432@gmail.com>
> 
> cls_flower keeps filter and mask state separately. After a filter is
> removed or replaced, some paths can still need the mask data associated
> with that filter.
> 
> Cache the mask key and dissector in struct cls_fl_filter when the mask
> is assigned, and use the cached copies in dump and offload paths. This
> avoids depending on the external mask object's lifetime after delete or
> replace.
> 
> Fixes: 92149190067d ("net: sched: flower: set unlocked flag for flower proto ops")
> Cc: stable@kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Yucheng Lu <kanolyc@gmail.com>
> Signed-off-by: Yuhang Zheng <z1652074432@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>

Jiri, do you have an opinion? Feels slightly wasteful (as sashiko points
out), IDK if there's a cleaner fix.

> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 099ff6a3e1f5..c1f10b4ec748 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -124,8 +124,10 @@ struct cls_fl_head {
>  
>  struct cls_fl_filter {
>  	struct fl_flow_mask *mask;
> +	struct flow_dissector mask_dissector;
>  	struct rhash_head ht_node;
>  	struct fl_flow_key mkey;
> +	struct fl_flow_key mask_key;
>  	struct tcf_exts exts;
>  	struct tcf_result res;
>  	struct fl_flow_key key;
> @@ -445,6 +447,12 @@ static void fl_destroy_filter_work(struct work_struct *work)
>  	__fl_destroy_filter(f);
>  }
>  
> +static void fl_filter_copy_mask(struct cls_fl_filter *f)
> +{
> +	f->mask_key = f->mask->key;
> +	f->mask_dissector = f->mask->dissector;
> +}
> +
>  static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f,
>  				 bool rtnl_held, struct netlink_ext_ack *extack)
>  {
> @@ -476,8 +484,8 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
>  	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, extack);
>  	cls_flower.command = FLOW_CLS_REPLACE;
>  	cls_flower.cookie = (unsigned long) f;
> -	cls_flower.rule->match.dissector = &f->mask->dissector;
> -	cls_flower.rule->match.mask = &f->mask->key;
> +	cls_flower.rule->match.dissector = &f->mask_dissector;
> +	cls_flower.rule->match.mask = &f->mask_key;
>  	cls_flower.rule->match.key = &f->mkey;
>  	cls_flower.classid = f->res.classid;
>  
> @@ -2489,6 +2497,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>  	err = fl_check_assign_mask(head, fnew, fold, mask);
>  	if (err)
>  		goto unbind_filter;
> +	fl_filter_copy_mask(fnew);
>  
>  	err = fl_ht_insert_unique(fnew, fold, &in_ht);
>  	if (err)
> @@ -2705,8 +2714,8 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb,
>  		cls_flower.command = add ?
>  			FLOW_CLS_REPLACE : FLOW_CLS_DESTROY;
>  		cls_flower.cookie = (unsigned long)f;
> -		cls_flower.rule->match.dissector = &f->mask->dissector;
> -		cls_flower.rule->match.mask = &f->mask->key;
> +		cls_flower.rule->match.dissector = &f->mask_dissector;
> +		cls_flower.rule->match.mask = &f->mask_key;
>  		cls_flower.rule->match.key = &f->mkey;
>  
>  		err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts,
> @@ -3709,7 +3718,7 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
>  		goto nla_put_failure_locked;
>  
>  	key = &f->key;
> -	mask = &f->mask->key;
> +	mask = &f->mask_key;
>  	skip_hw = tc_skip_hw(f->flags);
>  
>  	if (fl_dump_key(skb, net, key, mask))


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-23 18:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 16:03 [PATCH net v2 1/1] net/sched: cls_flower: avoid stale mask references after delete Ren Wei
2026-04-23 18:51 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox