| Message ID | 20230301-net-mlx5e-avoid-iter-after-loop-v1-1-064c0e9b1505@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [net-next] net/mlx5e: avoid usage of list iterator after loop | expand |
On Mon, Mar 13, 2023 at 04:26:02PM +0100, Jakob Koschel wrote: > 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(-) > Thanks, Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
On 13 Mar 16:26, Jakob Koschel wrote: >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> Applied to net-next-mlx5. Thanks.
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)) {
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(-) --- base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9 change-id: 20230301-net-mlx5e-avoid-iter-after-loop-dcc215275a96 Best regards,