Message ID | 20240220085928.9161-1-jianbol@nvidia.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 1fde0ca3a0de7e9f917668941156959dd5e9108b |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net] net/sched: flower: Add lock protection when remove filter handle | expand |
Tue, Feb 20, 2024 at 09:59:28AM CET, jianbol@nvidia.com wrote: >As IDR can't protect itself from the concurrent modification, place >idr_remove() under the protection of tp->lock. > >Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle initialization earlier") >Signed-off-by: Jianbo Liu <jianbol@nvidia.com> >Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> >Reviewed-by: Gal Pressman <gal@nvidia.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
On Tue, Feb 20, 2024 at 4:17 AM Jiri Pirko <jiri@resnulli.us> wrote: > > Tue, Feb 20, 2024 at 09:59:28AM CET, jianbol@nvidia.com wrote: > >As IDR can't protect itself from the concurrent modification, place > >idr_remove() under the protection of tp->lock. > > > >Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle initialization earlier") > >Signed-off-by: Jianbo Liu <jianbol@nvidia.com> > >Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> > >Reviewed-by: Gal Pressman <gal@nvidia.com> > > Reviewed-by: Jiri Pirko <jiri@nvidia.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Jianbo, do you have a new test case that caught this that we can use? Just curious why it hasnt been caught earlier (it's been there for about a year). cheers, jamal
On Tue, 2024-02-20 at 09:53 -0500, Jamal Hadi Salim wrote: > On Tue, Feb 20, 2024 at 4:17 AM Jiri Pirko <jiri@resnulli.us> wrote: > > > > Tue, Feb 20, 2024 at 09:59:28AM CET, jianbol@nvidia.com wrote: > > > As IDR can't protect itself from the concurrent modification, > > > place > > > idr_remove() under the protection of tp->lock. > > > > > > Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle > > > initialization earlier") > > > Signed-off-by: Jianbo Liu <jianbol@nvidia.com> > > > Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> > > > Reviewed-by: Gal Pressman <gal@nvidia.com> > > > > Reviewed-by: Jiri Pirko <jiri@nvidia.com> > > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> > > Jianbo, do you have a new test case that caught this that we can > use? > Just curious why it hasnt been caught earlier (it's been there for > about a year). I don't have the test case, sorry. The issue was caught when we stressed performance on BlueField-3. Thanks! Jianbo > > cheers, > jamal
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 20 Feb 2024 08:59:28 +0000 you wrote: > As IDR can't protect itself from the concurrent modification, place > idr_remove() under the protection of tp->lock. > > Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle initialization earlier") > Signed-off-by: Jianbo Liu <jianbol@nvidia.com> > Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> > Reviewed-by: Gal Pressman <gal@nvidia.com> > > [...] Here is the summary with links: - [net] net/sched: flower: Add lock protection when remove filter handle https://git.kernel.org/netdev/net/c/1fde0ca3a0de You are awesome, thank you!
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index bfedc3d4423d..e1314674b4a9 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -2460,8 +2460,11 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, } errout_idr: - if (!fold) + if (!fold) { + spin_lock(&tp->lock); idr_remove(&head->handle_idr, fnew->handle); + spin_unlock(&tp->lock); + } __fl_put(fnew); errout_tb: kfree(tb);