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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

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

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@...il.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@...il.com>