On 18/06/2026 15:48, Simon Horman wrote:

External email: Use caution opening links or attachments


This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev

Simon says: I think the first two issues flagged below warrant
             consideration within the context of this patch.

             But the third and final issue flagged below as pre-existing
             seems to be something to consider in the context of
             possible follow-up.
---
net/mlx5e: TC, skip peer flow cleanup when LAG seq is unavailable

This commit addresses a bad memory access crash by skipping peer flows when
mlx5_lag_get_dev_seq() returns an error. This occurs when the peer isn't in
the LAG or when no device is marked as master.

quoted

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index a9001d1c902ff..c6e6534a5e23b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c

@@ -2155,6 +2155,9 @@ static void mlx5e_tc_del_fdb_peers_flow(struct mlx5e_tc_flow *flow)
       devcom = flow->priv->mdev->priv.eswitch->devcom;
       mlx5_devcom_for_each_peer_entry(devcom, peer_esw, pos) {
               i = mlx5_lag_get_dev_seq(peer_esw->dev);
+             if (i < 0)
+                     continue;
+

[Severity: Critical]
Does skipping the remainder of this loop prevent the flow from being
unlinked from the doubly-linked list?

By using continue here in mlx5e_tc_del_fdb_peers_flow(), we skip calling
mlx5e_tc_del_fdb_peer_flow(), which means flow->peer[i] is never
unlinked from the esw->offloads.peer_flows[original_i] list.

only in case a LAG member is removed from ldev, mlx5_lag_get_dev_seq()
will return error.
before LAG member is removed, esw->devcom is cleanup, which invoke
mlx5e_tc_clean_fdb_peer_flows(), which remove all peer flows.
Hence, no flow remains.

If the flow is subsequently freed via mlx5e_flow_put(), will this leave a
freed list node resident in the active list and guarantee a use-after-free
list corruption during the next list traversal?

quoted

               mlx5e_tc_del_fdb_peer_flow(flow, i);
       }
  }

[ ... ]

quoted

@@ -5526,6 +5529,9 @@ void mlx5e_tc_clean_fdb_peer_flows(struct mlx5_eswitch *esw)

       mlx5_devcom_for_each_peer_entry(devcom, peer_esw, pos) {
               i = mlx5_lag_get_dev_seq(peer_esw->dev);
+             if (i < 0)
+                     continue;
+

[Severity: Critical]
Similarly to the above, does this skip unlinking during flow cleanup when
the LAG sequence is unavailable?

By the time get_dev_seq() can fail, the member is already out of ldev
- and clean_fdb_peer_flows() (devcom unpair) ran before that, while seq
was still valid, so the flows are already unlinked.
The guard just covers that later window; nothing remains to leak.

quoted

               list_for_each_entry_safe(flow, tmp, &esw->offloads.peer_flows[i], peer[i])
                       mlx5e_tc_del_fdb_peers_flow(flow);
       }

[Severity: Critical]
This isn't a bug introduced by this patch, but is there a similar missing
bounds check during flow addition in mlx5e_tc_add_fdb_peer_flow() in this
same file?

If a flow is added concurrently with LAG teardown, mlx5_lag_get_dev_seq()
can return -ENOENT (-2):

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:mlx5e_tc_add_fdb_peer_flow() {
     ...
     int i = mlx5_lag_get_dev_seq(peer_esw->dev);
     ...
     list_add_tail(&flow->peer[i], &esw->offloads.peer_flows[i]);
     ...
}

Does this result in an out-of-bounds write to a negative array index?

No, if LAG is destroyed, than is_peer_flow_needed will return false and
we won't enter here.
the whole peer loop runs under the devcom read lock
(mlx5_devcom_for_each_peer_begin), while devcom unpair - which is what
precedes LAG member removal and runs clean_fdb_peer_flows - takes the
write lock. The read lock therefore blocks teardown for the duration, so
mlx5_lag_get_dev_seq() can't go negative here.