Skip to content

Commit 64ea9e9

Browse files
Maxim Mikityanskiygregkh
authored andcommitted
net/mlx5e: kTLS, Use refcounts to free kTLS RX priv context
[ Upstream commit b850bbf ] wait_for_resync is unreliable - if it timeouts, priv_rx will be freed anyway. However, mlx5e_ktls_handle_get_psv_completion will be called sooner or later, leading to use-after-free. For example, it can happen if a CQ error happened, and ICOSQ stopped, but later on the queues are destroyed, and ICOSQ is flushed with mlx5e_free_icosq_descs. This patch converts the lifecycle of priv_rx to fully refcount-based, so that the struct won't be freed before the refcount goes to zero. Fixes: 0419d8c ("net/mlx5e: kTLS, Add kTLS RX resync support") Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com> Reviewed-by: Tariq Toukan <tariqt@nvidia.com> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 08b42b6 commit 64ea9e9

1 file changed

Lines changed: 30 additions & 34 deletions

File tree

  • drivers/net/ethernet/mellanox/mlx5/core/en_accel

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,20 @@ struct mlx5e_ktls_offload_context_rx {
5757
struct mlx5e_ktls_rx_resync_ctx resync;
5858
};
5959

60+
static bool mlx5e_ktls_priv_rx_put(struct mlx5e_ktls_offload_context_rx *priv_rx)
61+
{
62+
if (!refcount_dec_and_test(&priv_rx->resync.refcnt))
63+
return false;
64+
65+
kfree(priv_rx);
66+
return true;
67+
}
68+
69+
static void mlx5e_ktls_priv_rx_get(struct mlx5e_ktls_offload_context_rx *priv_rx)
70+
{
71+
refcount_inc(&priv_rx->resync.refcnt);
72+
}
73+
6074
static int mlx5e_ktls_create_tir(struct mlx5_core_dev *mdev, u32 *tirn, u32 rqtn)
6175
{
6276
int err, inlen;
@@ -326,15 +340,15 @@ static void resync_handle_work(struct work_struct *work)
326340
priv_rx = container_of(resync, struct mlx5e_ktls_offload_context_rx, resync);
327341

328342
if (unlikely(test_bit(MLX5E_PRIV_RX_FLAG_DELETING, priv_rx->flags))) {
329-
refcount_dec(&resync->refcnt);
343+
mlx5e_ktls_priv_rx_put(priv_rx);
330344
return;
331345
}
332346

333347
c = resync->priv->channels.c[priv_rx->rxq];
334348
sq = &c->async_icosq;
335349

336350
if (resync_post_get_progress_params(sq, priv_rx))
337-
refcount_dec(&resync->refcnt);
351+
mlx5e_ktls_priv_rx_put(priv_rx);
338352
}
339353

340354
static void resync_init(struct mlx5e_ktls_rx_resync_ctx *resync,
@@ -377,7 +391,11 @@ static int resync_handle_seq_match(struct mlx5e_ktls_offload_context_rx *priv_rx
377391
return err;
378392
}
379393

380-
/* Function is called with elevated refcount, it decreases it. */
394+
/* Function can be called with the refcount being either elevated or not.
395+
* It decreases the refcount and may free the kTLS priv context.
396+
* Refcount is not elevated only if tls_dev_del has been called, but GET_PSV was
397+
* already in flight.
398+
*/
381399
void mlx5e_ktls_handle_get_psv_completion(struct mlx5e_icosq_wqe_info *wi,
382400
struct mlx5e_icosq *sq)
383401
{
@@ -410,7 +428,7 @@ void mlx5e_ktls_handle_get_psv_completion(struct mlx5e_icosq_wqe_info *wi,
410428
tls_offload_rx_resync_async_request_end(priv_rx->sk, cpu_to_be32(hw_seq));
411429
priv_rx->stats->tls_resync_req_end++;
412430
out:
413-
refcount_dec(&resync->refcnt);
431+
mlx5e_ktls_priv_rx_put(priv_rx);
414432
dma_unmap_single(dev, buf->dma_addr, PROGRESS_PARAMS_PADDED_SIZE, DMA_FROM_DEVICE);
415433
kfree(buf);
416434
}
@@ -431,9 +449,9 @@ static bool resync_queue_get_psv(struct sock *sk)
431449
return false;
432450

433451
resync = &priv_rx->resync;
434-
refcount_inc(&resync->refcnt);
452+
mlx5e_ktls_priv_rx_get(priv_rx);
435453
if (unlikely(!queue_work(resync->priv->tls->rx_wq, &resync->work)))
436-
refcount_dec(&resync->refcnt);
454+
mlx5e_ktls_priv_rx_put(priv_rx);
437455

438456
return true;
439457
}
@@ -625,31 +643,6 @@ int mlx5e_ktls_add_rx(struct net_device *netdev, struct sock *sk,
625643
return err;
626644
}
627645

628-
/* Elevated refcount on the resync object means there are
629-
* outstanding operations (uncompleted GET_PSV WQEs) that
630-
* will read the resync / priv_rx objects once completed.
631-
* Wait for them to avoid use-after-free.
632-
*/
633-
static void wait_for_resync(struct net_device *netdev,
634-
struct mlx5e_ktls_rx_resync_ctx *resync)
635-
{
636-
#define MLX5E_KTLS_RX_RESYNC_TIMEOUT 20000 /* msecs */
637-
unsigned long exp_time = jiffies + msecs_to_jiffies(MLX5E_KTLS_RX_RESYNC_TIMEOUT);
638-
unsigned int refcnt;
639-
640-
do {
641-
refcnt = refcount_read(&resync->refcnt);
642-
if (refcnt == 1)
643-
return;
644-
645-
msleep(20);
646-
} while (time_before(jiffies, exp_time));
647-
648-
netdev_warn(netdev,
649-
"Failed waiting for kTLS RX resync refcnt to be released (%u).\n",
650-
refcnt);
651-
}
652-
653646
void mlx5e_ktls_del_rx(struct net_device *netdev, struct tls_context *tls_ctx)
654647
{
655648
struct mlx5e_ktls_offload_context_rx *priv_rx;
@@ -671,14 +664,17 @@ void mlx5e_ktls_del_rx(struct net_device *netdev, struct tls_context *tls_ctx)
671664
wait_for_completion(&priv_rx->add_ctx);
672665
resync = &priv_rx->resync;
673666
if (cancel_work_sync(&resync->work))
674-
refcount_dec(&resync->refcnt);
675-
wait_for_resync(netdev, resync);
667+
mlx5e_ktls_priv_rx_put(priv_rx);
676668

677669
priv_rx->stats->tls_del++;
678670
if (priv_rx->rule.rule)
679671
mlx5e_accel_fs_del_sk(priv_rx->rule.rule);
680672

681673
mlx5_core_destroy_tir(mdev, priv_rx->tirn);
682674
mlx5_ktls_destroy_key(mdev, priv_rx->key_id);
683-
kfree(priv_rx);
675+
/* priv_rx should normally be freed here, but if there is an outstanding
676+
* GET_PSV, deallocation will be delayed until the CQE for GET_PSV is
677+
* processed.
678+
*/
679+
mlx5e_ktls_priv_rx_put(priv_rx);
684680
}

0 commit comments

Comments
 (0)