[PATCH net-next] net/mlx5e: avoid usage of list iterator after loop

From: Jakob Koschel <jkl820.git@gmail.com>
To: Boris Pismenny <borisp@nvidia.com>,
	Saeed Mahameed <saeedm@nvidia.com>,
	Leon Romanovsky <leon@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Pietro Borrello <borrello@diag.uniroma1.it>,
	Cristiano Giuffrida <c.giuffrida@vu.nl>,
	"Bos, H.J." <h.j.bos@vu.nl>, Jakob Koschel <jkl820.git@gmail.com>
Subject: [PATCH net-next] net/mlx5e: avoid usage of list iterator after loop
Date: Mon, 13 Mar 2023 16:26:02 +0100	[thread overview]
Message-ID: <20230301-net-mlx5e-avoid-iter-after-loop-v1-1-064c0e9b1505@gmail.com> (raw)

If potentially no valid element is found, 'priv_rx' would contain an
invalid pointer past the iterator loop. To ensure 'priv_rx' is always
valid, we only set it if the correct element was found. That allows
adding a WARN_ON() in case the code works incorrectly, exposing
currently undetectable potential bugs.

Additionally, Linus proposed to avoid any use of the list iterator
variable after the loop, in the attempt to move the list iterator
variable declaration into the macro to avoid any potential misuse after
the loop [1].

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jkl820.git@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
index 4be770443b0c..8aad500e622d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
@@ -718,7 +718,7 @@ void mlx5e_ktls_del_rx(struct net_device *netdev, struct tls_context *tls_ctx)
 
 bool mlx5e_ktls_rx_handle_resync_list(struct mlx5e_channel *c, int budget)
 {
-	struct mlx5e_ktls_offload_context_rx *priv_rx, *tmp;
+	struct mlx5e_ktls_offload_context_rx *priv_rx = NULL, *iter, *tmp;
 	struct mlx5e_ktls_resync_resp *ktls_resync;
 	struct mlx5_wqe_ctrl_seg *db_cseg;
 	struct mlx5e_icosq *sq;
@@ -735,10 +735,12 @@ bool mlx5e_ktls_rx_handle_resync_list(struct mlx5e_channel *c, int budget)
 	i = 0;
 
 	spin_lock(&ktls_resync->lock);
-	list_for_each_entry_safe(priv_rx, tmp, &ktls_resync->list, list) {
-		list_move(&priv_rx->list, &local_list);
-		if (++i == budget)
+	list_for_each_entry_safe(iter, tmp, &ktls_resync->list, list) {
+		list_move(&iter->list, &local_list);
+		if (++i == budget) {
+			priv_rx = iter;
 			break;
+		}
 	}
 	if (list_empty(&ktls_resync->list))
 		clear_bit(MLX5E_SQ_STATE_PENDING_TLS_RX_RESYNC, &sq->state);
@@ -765,6 +767,7 @@ bool mlx5e_ktls_rx_handle_resync_list(struct mlx5e_channel *c, int budget)
 		mlx5e_notify_hw(&sq->wq, sq->pc, sq->uar_map, db_cseg);
 	spin_unlock(&c->async_icosq_lock);
 
+	WARN_ON(!priv_rx);
 	priv_rx->rq_stats->tls_resync_res_ok += j;
 
 	if (!list_empty(&local_list)) {

---
base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9
change-id: 20230301-net-mlx5e-avoid-iter-after-loop-dcc215275a96

Best regards,
-- 
Jakob Koschel <jkl820.git@gmail.com>

