[PATCH net] net/sched: flower: Fix wrong handle assignment during filter change

[PATCH net] net/sched: flower: Fix wrong handle assignment during filter change

[Ivan Vecera]

Commit 08a0063df3ae ("net/sched: flower: Move filter handle initialization
earlier") moved filter handle initialization but an assignment of
the handle to fnew->handle is done regardless of fold value. This is wrong
because if fold != NULL (so fold->handle == handle) no new handle is
allocated and passed handle is assigned to fnew->handle. Then if any
subsequent action in fl_change() fails then the handle value is
removed from IDR that is incorrect as we will have still valid old filter
instance with handle that is not present in IDR.
Fix this issue by moving the assignment so it is done only when passed
fold == NULL.

Prior the patch:
[root@machine tc-testing]# ./tdc.py -d enp1s0f0np0 -e 14be
Test 14be: Concurrently replace same range of 100k flower filters from 10 tc instances
exit: 123
exit: 0
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Command failed tmp/replace_6:1885


All test results:

1..1
not ok 1 14be - Concurrently replace same range of 100k flower filters from 10 tc instances
        Command exited with 123, expected 0
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Command failed tmp/replace_6:1885

After the patch:
[root@machine tc-testing]# ./tdc.py -d enp1s0f0np0 -e 14be
Test 14be: Concurrently replace same range of 100k flower filters from 10 tc instances

All test results:

1..1
ok 1 14be - Concurrently replace same range of 100k flower filters from 10 tc instances

Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle initialization earlier")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 net/sched/cls_flower.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 475fe222a855..fa6c2bb0b626 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -2231,8 +2231,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 			kfree(fnew);
 			goto errout_tb;
 		}
+		fnew->handle = handle;
 	}
-	fnew->handle = handle;
 
 	err = tcf_exts_init_ex(&fnew->exts, net, TCA_FLOWER_ACT, 0, tp, handle,
 			       !tc_skip_hw(fnew->flags));
-- 
2.39.1

Re: [PATCH net] net/sched: flower: Fix wrong handle assignment during filter change

[Simon Horman]

On Tue, Apr 25, 2023 at 04:06:04PM +0200, Ivan Vecera wrote:
> Commit 08a0063df3ae ("net/sched: flower: Move filter handle initialization
> earlier") moved filter handle initialization but an assignment of
> the handle to fnew->handle is done regardless of fold value. This is wrong
> because if fold != NULL (so fold->handle == handle) no new handle is
> allocated and passed handle is assigned to fnew->handle. Then if any
> subsequent action in fl_change() fails then the handle value is
> removed from IDR that is incorrect as we will have still valid old filter
> instance with handle that is not present in IDR.
> Fix this issue by moving the assignment so it is done only when passed
> fold == NULL.
> 
> Prior the patch:
> [root@machine tc-testing]# ./tdc.py -d enp1s0f0np0 -e 14be
> Test 14be: Concurrently replace same range of 100k flower filters from 10 tc instances
> exit: 123
> exit: 0
> RTNETLINK answers: Invalid argument
> We have an error talking to the kernel
> Command failed tmp/replace_6:1885
> 
> 
> All test results:
> 
> 1..1
> not ok 1 14be - Concurrently replace same range of 100k flower filters from 10 tc instances
>         Command exited with 123, expected 0
> RTNETLINK answers: Invalid argument
> We have an error talking to the kernel
> Command failed tmp/replace_6:1885
> 
> After the patch:
> [root@machine tc-testing]# ./tdc.py -d enp1s0f0np0 -e 14be
> Test 14be: Concurrently replace same range of 100k flower filters from 10 tc instances
> 
> All test results:
> 
> 1..1
> ok 1 14be - Concurrently replace same range of 100k flower filters from 10 tc instances
> 
> Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle initialization earlier")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net] net/sched: flower: Fix wrong handle assignment during filter change

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 25 Apr 2023 16:06:04 +0200 you wrote:
> Commit 08a0063df3ae ("net/sched: flower: Move filter handle initialization
> earlier") moved filter handle initialization but an assignment of
> the handle to fnew->handle is done regardless of fold value. This is wrong
> because if fold != NULL (so fold->handle == handle) no new handle is
> allocated and passed handle is assigned to fnew->handle. Then if any
> subsequent action in fl_change() fails then the handle value is
> removed from IDR that is incorrect as we will have still valid old filter
> instance with handle that is not present in IDR.
> Fix this issue by moving the assignment so it is done only when passed
> fold == NULL.
> 
> [...]

Here is the summary with links:
  - [net] net/sched: flower: Fix wrong handle assignment during filter change
    https://git.kernel.org/netdev/net/c/32eff6bacec2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html