[PATCH] net: microchip: vcap: stop scanning after deleting key field

[PATCH] net: microchip: vcap: stop scanning after deleting key field

[Harshit Mogalapalli]

The only call outside the KUnit tests is in
sparx5_tc_add_rule_copy(), which passes a fixed key list with no
duplicate entries. Therefore, the potential double-free is not
currently reachable in tree.

However, vcap_filter_rule_keys() is exported and does not require its
key list entries to be unique. If a future or external caller supplies
the same key at another position that the inner loop visits, it calls
list_del() and kfree() a second time on the freed field, leading to a
potential double-free.

Break the inner loop after removing a matching field. The outer safe
list traversal continues to process the remaining rule fields.

Fixes: 465a38a269e9 ("net: microchip: sparx5: Support for copying and modifying rules in the API")
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/all/ahs-lCAXXikkaHky@stanley.mountain/
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
 drivers/net/ethernet/microchip/vcap/vcap_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index 788c0728d763..0f8d98f2094d 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -3527,8 +3527,8 @@ int vcap_filter_rule_keys(struct vcap_rule *rule,
 				if (key == keylist[idx]) {
 					list_del(&ckf->ctrl.list);
 					kfree(ckf);
-					idx++;
 					err = 0;
+					break;
 				}
 		}
 	}
-- 
2.52.0

Re: [PATCH] net: microchip: vcap: stop scanning after deleting key field

[Daniel Machon]

> The only call outside the KUnit tests is in
> sparx5_tc_add_rule_copy(), which passes a fixed key list with no
> duplicate entries. Therefore, the potential double-free is not
> currently reachable in tree.

Agree. Not reachable today.

> 
> However, vcap_filter_rule_keys() is exported and does not require its
> key list entries to be unique. If a future or external caller supplies
> the same key at another position that the inner loop visits, it calls
> list_del() and kfree() a second time on the freed field, leading to a
> potential double-free.
> 

Also agree.

> Break the inner loop after removing a matching field. The outer safe
> list traversal continues to process the remaining rule fields.
> 
> Fixes: 465a38a269e9 ("net: microchip: sparx5: Support for copying and modifying rules in the API")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/all/ahs-lCAXXikkaHky@stanley.mountain/
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
>  drivers/net/ethernet/microchip/vcap/vcap_api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
> index 788c0728d763..0f8d98f2094d 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
> @@ -3527,8 +3527,8 @@ int vcap_filter_rule_keys(struct vcap_rule *rule,
>                                 if (key == keylist[idx]) {
>                                         list_del(&ckf->ctrl.list);
>                                         kfree(ckf);
> -                                       idx++;
>                                         err = 0;
> +                                       break;
>                                 }
>                 }
>         }
> --
> 2.52.0
> 

Patch is missing the target tree (net), the fix LGTM.

Reviewed-by: Daniel Machon <daniel.machon@microchip.com>

Re: [PATCH] net: microchip: vcap: stop scanning after deleting key field

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 24 Sep 2026 23:39:36 -0700 you wrote:
> The only call outside the KUnit tests is in
> sparx5_tc_add_rule_copy(), which passes a fixed key list with no
> duplicate entries. Therefore, the potential double-free is not
> currently reachable in tree.
> 
> However, vcap_filter_rule_keys() is exported and does not require its
> key list entries to be unique. If a future or external caller supplies
> the same key at another position that the inner loop visits, it calls
> list_del() and kfree() a second time on the freed field, leading to a
> potential double-free.
> 
> [...]

Here is the summary with links:
  - net: microchip: vcap: stop scanning after deleting key field
    https://git.kernel.org/netdev/net/c/99b43ede9e35

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