[PATCH net] net/mlx5e: TC, Fix double free of branch in failure

[PATCH net] net/mlx5e: TC, Fix double free of branch in failure

[Thomas Fourier]

When allocating the false branch fails in parse_branch_ctrl(), the
already allocated true branch is freed but attr->branch_true is left
pointing to it. The caller parse_tc_actions() fails the flow, and
free_flow_post_acts(), mlx5_free_flow_attr_actions() which calls again
free_branch_attr() on attr->branch_true.

Clear the pointer after freeing it.

This bug is theoretical and was compile tested only.

Fixes: f86488cb4685 ("net/mlx5e: TC, initialize branch flow attributes")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index b290beb4369a..c40c7b55a54d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4005,6 +4005,7 @@ parse_branch_ctrl(struct flow_action_entry *act, struct mlx5e_tc_act *tc_act,
 
 err_branch_false:
 	free_branch_attr(flow, attr->branch_true);
+	attr->branch_true = NULL;
 out_err:
 	return err;
 }
-- 
2.53.0

Re: [PATCH net] net/mlx5e: TC, Fix double free of branch in failure

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net/mlx5e: TC, Fix double free of branch in failure

[Paolo Abeni]

On 10/28/26 15:57, Thomas Fourier wrote:
> When allocating the false branch fails in parse_branch_ctrl(), the
> already allocated true branch is freed but attr->branch_true is left
> pointing to it. The caller parse_tc_actions() fails the flow, and
> free_flow_post_acts(), mlx5_free_flow_attr_actions() which calls again
> free_branch_attr() on attr->branch_true.
> 
> Clear the pointer after freeing it.
> 
> This bug is theoretical and was compile tested only.
> 
> Fixes: f86488cb4685 ("net/mlx5e: TC, initialize branch flow attributes")
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
Given the lack of H/W testing I would appreciate an explicit ack
from the mlx crew.

Thanks,

Paolo

Re: [PATCH net] net/mlx5e: TC, Fix double free of branch in failure

[Tariq Toukan]

On 05/11/2026 11:18, Paolo Abeni wrote:
> On 10/28/26 15:57, Thomas Fourier wrote:
>> When allocating the false branch fails in parse_branch_ctrl(), the
>> already allocated true branch is freed but attr->branch_true is left
>> pointing to it. The caller parse_tc_actions() fails the flow, and
>> free_flow_post_acts(), mlx5_free_flow_attr_actions() which calls again
>> free_branch_attr() on attr->branch_true.
>>
>> Clear the pointer after freeing it.
>>
>> This bug is theoretical and was compile tested only.
>>

I reproduced it on ConnectX-6 Dx in switchdev mode, with KASAN:

     tc filter add dev $UPLINK ingress protocol ip flower skip_sw \
             dst_ip 192.0.2.2 action police rate 1mbit burst 20k \
             conform-exceed jump 1 / jump 1 action pass action drop

Every attempt reports a double-free in mlx5_free_flow_attr_actions().
With this patch, 50 attempts reported none.

>> Fixes: f86488cb4685 ("net/mlx5e: TC, initialize branch flow attributes")
>> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> Given the lack of H/W testing I would appreciate an explicit ack
> from the mlx crew.
> 
> Thanks,
> 
> Paolo

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Tested-by: Tariq Toukan <tariqt@nvidia.com>

The same command also leaks the parse_attr of the false branch, as 
alloc_branch_attr() frees only the attr on its error path. I'll send a 
separate fix for that.

Thanks,
Tariq