mlxsw_sp_fid_port_vid_list_add() inserts into a list sorted by
local_port. It walks the list to find the first entry with a
larger local_port, then inserts the new entry before it:

    list_add_tail(&port_vid->list, &tmp_port_vid->list);

If the loop falls through (the new local_port is the largest),
tmp_port_vid runs off the end of the list. &tmp_port_vid->list
then ends up at the list head itself (container_of() offsets
cancel), and list_add_tail() inserts at the tail. So the code
works today.

It is fragile though. Anyone who later adds a read of another
field of tmp_port_vid will hit memory outside the list head.

Track the insertion point with a dedicated list_head pointer.
Initialise insert_before to &fid->port_vid_list, set it to
&tmp_port_vid->list only on early break, and pass insert_before
to list_add_tail(). The cursor is no longer touched after the
loop. Behaviour is unchanged.

Same shape as the Koschel cleanups from 2022 (e.g. 99d8ae4ec8a
tracing, 2966a9918df clockevents, dc1acd5c946 dlm).

Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c
index 4f921bbc1e77..8d6cea43be61 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c
@@ -1021,6 +1021,7 @@ mlxsw_sp_fid_port_vid_list_add(struct mlxsw_sp_fid *fid, u16 local_port,
 			       u16 vid)
 {
 	struct mlxsw_sp_fid_port_vid *port_vid, *tmp_port_vid;
+	struct list_head *insert_before = &fid->port_vid_list;
 
 	port_vid = kzalloc_obj(*port_vid);
 	if (!port_vid)
@@ -1030,11 +1031,13 @@ mlxsw_sp_fid_port_vid_list_add(struct mlxsw_sp_fid *fid, u16 local_port,
 	port_vid->vid = vid;
 
 	list_for_each_entry(tmp_port_vid, &fid->port_vid_list, list) {
-		if (tmp_port_vid->local_port > local_port)
+		if (tmp_port_vid->local_port > local_port) {
+			insert_before = &tmp_port_vid->list;
 			break;
+		}
 	}
 
-	list_add_tail(&port_vid->list, &tmp_port_vid->list);
+	list_add_tail(&port_vid->list, insert_before);
 	return 0;
 }
 
-- 
2.34.1

On Mon, May 25, 2026 at 03:17:59PM +0800, Maoyi Xie wrote:
> mlxsw_sp_fid_port_vid_list_add() inserts into a list sorted by
> local_port. It walks the list to find the first entry with a
> larger local_port, then inserts the new entry before it:
> 
>     list_add_tail(&port_vid->list, &tmp_port_vid->list);
> 
> If the loop falls through (the new local_port is the largest),
> tmp_port_vid runs off the end of the list. &tmp_port_vid->list
> then ends up at the list head itself (container_of() offsets
> cancel), and list_add_tail() inserts at the tail. So the code
> works today.
> 
> It is fragile though. Anyone who later adds a read of another
> field of tmp_port_vid will hit memory outside the list head.
> 
> Track the insertion point with a dedicated list_head pointer.
> Initialise insert_before to &fid->port_vid_list, set it to
> &tmp_port_vid->list only on early break, and pass insert_before
> to list_add_tail(). The cursor is no longer touched after the
> loop. Behaviour is unchanged.
> 
> Same shape as the Koschel cleanups from 2022 (e.g. 99d8ae4ec8a
> tracing, 2966a9918df clockevents, dc1acd5c946 dlm).
> 
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>