diff mbox series

[SRU,J:linux-bluefield,1/1] net/sched: flower: Add lock protection when remove filter handle

Message ID 20240226230230.86102-2-witu@nvidia.com
State New
Headers show
Series net/sched: flower: Add lock protection when remove filter handle | expand

Commit Message

William Tu Feb. 26, 2024, 11:02 p.m. UTC
From: Jianbo Liu <jianbol@nvidia.com>

BugLink: https://bugs.launchpad.net/bugs/2055052

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>
Link: https://lore.kernel.org/r/20240220085928.9161-1-jianbol@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(Backported from commit 1fde0ca3a0de7e9f917668941156959dd5e9108b)
[witu: resolve trivial conflicts in net/sched/cls_flower.c]
Signed-off-by: William Tu <witu@nvidia.com>
---
 net/sched/cls_flower.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Bartlomiej Zolnierkiewicz Feb. 27, 2024, 2:49 p.m. UTC | #1
Acked-by: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>

On Tue, Feb 27, 2024 at 12:03 AM William Tu <witu@nvidia.com> wrote:
>
> From: Jianbo Liu <jianbol@nvidia.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2055052
>
> 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>
> Link: https://lore.kernel.org/r/20240220085928.9161-1-jianbol@nvidia.com
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> (Backported from commit 1fde0ca3a0de7e9f917668941156959dd5e9108b)
> [witu: resolve trivial conflicts in net/sched/cls_flower.c]
> Signed-off-by: William Tu <witu@nvidia.com>
> ---
>  net/sched/cls_flower.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 323d8bf02fa6..a2361b632a09 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -2196,9 +2196,11 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>  errout_mask:
>         fl_mask_put(head, fnew->mask);
>  errout_idr:
> -       if (!fold)
> +       if (!fold) {
> +               spin_lock(&tp->lock);
>                 idr_remove(&head->handle_idr, fnew->handle);
> -errout:
> +               spin_unlock(&tp->lock);
> +       }
>         __fl_put(fnew);
>  errout_tb:
>         kfree(tb);
Andrei Gherzan March 1, 2024, 12:04 p.m. UTC | #2
On 24/02/26 03:02PM, William Tu wrote:
> From: Jianbo Liu <jianbol@nvidia.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2055052
> 
> 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>
> Link: https://lore.kernel.org/r/20240220085928.9161-1-jianbol@nvidia.com
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> (Backported from commit 1fde0ca3a0de7e9f917668941156959dd5e9108b)
> [witu: resolve trivial conflicts in net/sched/cls_flower.c]
> Signed-off-by: William Tu <witu@nvidia.com>
> ---
>  net/sched/cls_flower.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 323d8bf02fa6..a2361b632a09 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -2196,9 +2196,11 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>  errout_mask:
>  	fl_mask_put(head, fnew->mask);
>  errout_idr:
> -	if (!fold)
> +	if (!fold) {
> +		spin_lock(&tp->lock);
>  		idr_remove(&head->handle_idr, fnew->handle);
> -errout:

This label drop shouldn't have been included in a backport of
1fde0ca3a0de7e9f917668941156959dd5e9108b. This was mistakenly introduced
in our tree (0b8cb39ebe410f30f052cb126d08b1d0210cc1fa), and I would have
treated it as a "fixes 0b8cb39ebe410f30f052cb126d08b1d0210cc1fa"
separate commit.

I won't hold onto my ack for this, leaving it at the discretion of the
maintainer.

> +		spin_unlock(&tp->lock);
> +	}
>  	__fl_put(fnew);
>  errout_tb:
>  	kfree(tb);

Acked-by: Andrei Gherzan <andrei.gherzan@canonical.com>
diff mbox series

Patch

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 323d8bf02fa6..a2361b632a09 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -2196,9 +2196,11 @@  static int fl_change(struct net *net, struct sk_buff *in_skb,
 errout_mask:
 	fl_mask_put(head, fnew->mask);
 errout_idr:
-	if (!fold)
+	if (!fold) {
+		spin_lock(&tp->lock);
 		idr_remove(&head->handle_idr, fnew->handle);
-errout:
+		spin_unlock(&tp->lock);
+	}
 	__fl_put(fnew);
 errout_tb:
 	kfree(tb);