[PATCH net-next v1 0/4] net/mlx5: Don't free CQs whose destroy failed

[PATCH net-next v1 0/4] net/mlx5: Don't free CQs whose destroy failed

[Yuho Choi]

mlx5_core_destroy_cq() can fail, and when it does the CQ may still
exist in firmware with access to its buffer and doorbell record. It
also returns early in that case, without waiting for EQ handlers that
may already hold a reference to the CQ. The four helpers in this series
ignore that error and free the CQ buffer anyway, while their teardown
paths also free the object that embeds the struct mlx5_core_cq.

Keep the buffer and the object that embeds the CQ when DESTROY_CQ
fails, as the regular mlx5_ib CQ destroy path does. The HWS send ring
CQs are polled and never armed, so the HWS patch keeps only their
buffer rather than passing the error up through the queue open and
close paths.

The issues were found with a static analysis tool from ongoing, not yet
published research, and confirmed by code inspection. None of them was
reproduced, so this targets net-next; I can resend for net if you
prefer.

Compile-tested only (x86_64 defconfig with MLX5_CORE_EN, MLX5_ESWITCH,
MLX5_SW_STEERING and MLX5_HW_STEERING, W=1).

Yuho Choi (4):
  net/mlx5: Don't free the ASO CQ if destroying it fails
  net/mlx5: Don't free the WC test CQ if destroying it fails
  net/mlx5: DR, Don't free the send ring CQ if destroying it fails
  net/mlx5: HWS, Don't free the send ring CQ buffer if destroying the CQ
    fails

 .../net/ethernet/mellanox/mlx5/core/lib/aso.c  | 18 +++++++++++++-----
 .../mellanox/mlx5/core/steering/hws/send.c     |  4 +++-
 .../mellanox/mlx5/core/steering/sws/dr_send.c  |  4 +++-
 drivers/net/ethernet/mellanox/mlx5/core/wc.c   | 17 ++++++++++++-----
 4 files changed, 31 insertions(+), 12 deletions(-)


base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
-- 
2.43.0

[PATCH net-next v1 1/4] net/mlx5: Don't free the ASO CQ if destroying it fails

[Yuho Choi]

mlx5_aso_destroy_cq() ignores the result of mlx5_core_destroy_cq() and
frees the CQ buffer and doorbell record anyway, and its callers then
free the struct mlx5_aso that embeds the CQ. If DESTROY_CQ fails, the
CQ may still exist in firmware and retain access to its buffer and
doorbell record. mlx5_core_destroy_cq() also returns early in that case,
without waiting for EQ handlers that may already hold a reference to
the CQ.

Return the error from mlx5_aso_destroy_cq() and keep both the buffer
and the struct mlx5_aso when it fails, as the regular mlx5_ib CQ destroy
path does. A device in internal error is not affected, since the mlx5
core reports success for DESTROY_CQ in that state.

Fixes: cdd04f4d4d71 ("net/mlx5: Add support to create SQ and CQ for ASO")
Assisted-by: LLM
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
This was found with a static analysis tool from ongoing, not yet
published research, and confirmed by code inspection. It has not been
reproduced on hardware.

 .../net/ethernet/mellanox/mlx5/core/lib/aso.c  | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c
index 614cd57a40d74..6a9d31d539e9a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c
@@ -112,10 +112,14 @@ static int create_aso_cq(struct mlx5_aso_cq *cq, void *cqc_data)
 	return err;
 }
 
-static void mlx5_aso_destroy_cq(struct mlx5_aso_cq *cq)
+static int mlx5_aso_destroy_cq(struct mlx5_aso_cq *cq)
 {
-	mlx5_core_destroy_cq(cq->mdev, &cq->mcq);
-	mlx5_wq_destroy(&cq->wq_ctrl);
+	int err = mlx5_core_destroy_cq(cq->mdev, &cq->mcq);
+
+	/* The device may still access an undestroyed CQ's buffer; keep it. */
+	if (!err)
+		mlx5_wq_destroy(&cq->wq_ctrl);
+	return err;
 }
 
 static int mlx5_aso_create_cq(struct mlx5_core_dev *mdev, int numa_node,
@@ -326,7 +330,9 @@ struct mlx5_aso *mlx5_aso_create(struct mlx5_core_dev *mdev, u32 pdn)
 	return aso;
 
 err_sq:
-	mlx5_aso_destroy_cq(&aso->cq);
+	/* An undestroyed CQ may still be in use; keep the ASO that holds it. */
+	if (mlx5_aso_destroy_cq(&aso->cq))
+		return ERR_PTR(err);
 err_cq:
 	kfree(aso);
 	return ERR_PTR(err);
@@ -335,7 +341,9 @@ struct mlx5_aso *mlx5_aso_create(struct mlx5_core_dev *mdev, u32 pdn)
 void mlx5_aso_destroy(struct mlx5_aso *aso)
 {
 	mlx5_aso_destroy_sq(aso);
-	mlx5_aso_destroy_cq(&aso->cq);
+	/* An undestroyed CQ may still be in use; keep the ASO that holds it. */
+	if (mlx5_aso_destroy_cq(&aso->cq))
+		return;
 	kfree(aso);
 }
 
-- 
2.43.0

[PATCH net-next v1 2/4] net/mlx5: Don't free the WC test CQ if destroying it fails

[Yuho Choi]

mlx5_wc_destroy_cq() ignores the result of mlx5_core_destroy_cq() and
frees the CQ buffer and doorbell record anyway, and mlx5_core_test_wc()
then frees the struct mlx5_wc_sq that embeds the CQ. If DESTROY_CQ
fails, the CQ may still exist in firmware and retain access to its
buffer and doorbell record. mlx5_core_destroy_cq() also returns early in
that case, without waiting for EQ handlers that may already hold a
reference to the CQ.

Return the error from mlx5_wc_destroy_cq() and keep both the buffer and
the struct mlx5_wc_sq when it fails, as the regular mlx5_ib CQ destroy
path does. A device in internal error is not affected, since the mlx5
core reports success for DESTROY_CQ in that state.

Fixes: d98995b4bf98 ("net/mlx5: Reimplement write combining test")
Assisted-by: LLM
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
This was found with a static analysis tool from ongoing, not yet
published research, and confirmed by code inspection. It has not been
reproduced on hardware.

 drivers/net/ethernet/mellanox/mlx5/core/wc.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/wc.c b/drivers/net/ethernet/mellanox/mlx5/core/wc.c
index 7d3d4d739e2cd..df1b1555ed8f4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/wc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/wc.c
@@ -148,10 +148,14 @@ static int mlx5_wc_create_cq(struct mlx5_core_dev *mdev, struct mlx5_wc_cq *cq)
 	return err;
 }
 
-static void mlx5_wc_destroy_cq(struct mlx5_wc_cq *cq)
+static int mlx5_wc_destroy_cq(struct mlx5_wc_cq *cq)
 {
-	mlx5_core_destroy_cq(cq->mdev, &cq->mcq);
-	mlx5_wq_destroy(&cq->wq_ctrl);
+	int err = mlx5_core_destroy_cq(cq->mdev, &cq->mcq);
+
+	/* The device may still access an undestroyed CQ's buffer; keep it. */
+	if (!err)
+		mlx5_wq_destroy(&cq->wq_ctrl);
+	return err;
 }
 
 static int create_wc_sq(struct mlx5_core_dev *mdev, void *sqc_data,
@@ -360,6 +364,7 @@ static int mlx5_wc_poll_cq(struct mlx5_wc_sq *sq)
 static void mlx5_core_test_wc(struct mlx5_core_dev *mdev)
 {
 	unsigned int offset = 0;
+	bool keep_sq = false;
 	struct mlx5_wc_sq *sq;
 	int i, err;
 
@@ -396,11 +401,13 @@ static void mlx5_core_test_wc(struct mlx5_core_dev *mdev)
 	mlx5_wc_destroy_sq(sq);
 
 err_create_sq:
-	mlx5_wc_destroy_cq(&sq->cq);
+	/* An undestroyed CQ may still be in use; keep the SQ that holds it. */
+	keep_sq = mlx5_wc_destroy_cq(&sq->cq);
 err_create_cq:
 	mlx5_free_bfreg(mdev, &sq->bfreg);
 err_alloc_bfreg:
-	kfree(sq);
+	if (!keep_sq)
+		kfree(sq);
 
 	if (mdev->wc_state == MLX5_WC_STATE_UNSUPPORTED)
 		mlx5_core_warn(mdev, "Write combining is not supported\n");
-- 
2.43.0

[PATCH net-next v1 3/4] net/mlx5: DR, Don't free the send ring CQ if destroying it fails

[Yuho Choi]

dr_destroy_cq() ignores the result of mlx5_core_destroy_cq() and frees
the CQ buffer, doorbell record and struct mlx5dr_cq anyway. If
DESTROY_CQ fails, the CQ may still exist in firmware and retain access
to its buffer and doorbell record. mlx5_core_destroy_cq() also returns
early in that case, without waiting for EQ handlers that may already
hold a reference to the CQ.

Keep the CQ and its buffer when DESTROY_CQ fails, as the regular mlx5_ib
CQ destroy path does. A device in internal error is not affected, since
the mlx5 core reports success for DESTROY_CQ in that state.

Fixes: 297cccebdc5a ("net/mlx5: DR, Expose an internal API to issue RDMA operations")
Assisted-by: LLM
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
This was found with a static analysis tool from ongoing, not yet
published research, and confirmed by code inspection. It has not been
reproduced on hardware.

 .../net/ethernet/mellanox/mlx5/core/steering/sws/dr_send.c    | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_send.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_send.c
index 79e3c29d1044e..8fc41adb4c756 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_send.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_send.c
@@ -1130,7 +1130,9 @@ static struct mlx5dr_cq *dr_create_cq(struct mlx5_core_dev *mdev,
 
 static void dr_destroy_cq(struct mlx5_core_dev *mdev, struct mlx5dr_cq *cq)
 {
-	mlx5_core_destroy_cq(mdev, &cq->mcq);
+	/* The device may still use an undestroyed CQ; keep it. */
+	if (mlx5_core_destroy_cq(mdev, &cq->mcq))
+		return;
 	mlx5_wq_destroy(&cq->wq_ctrl);
 	kfree(cq);
 }
-- 
2.43.0

[PATCH net-next v1 4/4] net/mlx5: HWS, Don't free the send ring CQ buffer if destroying the CQ fails

[Yuho Choi]

hws_send_ring_close_cq() ignores the result of mlx5_core_destroy_cq()
and frees the CQ buffer and doorbell record with mlx5_wq_destroy()
anyway. If DESTROY_CQ fails, the CQ may still exist in firmware and
retain access to that memory after it has been returned to the
allocator.

Leak the buffer instead when the CQ cannot be destroyed. A device in
internal error is not affected, since the mlx5 core reports success for
DESTROY_CQ in that state.

Fixes: 2ca62599aa0b ("net/mlx5: HWS, added send engine and context handling")
Assisted-by: LLM
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
This was found with a static analysis tool from ongoing, not yet
published research, and confirmed by code inspection. It has not been
reproduced on hardware.

 drivers/net/ethernet/mellanox/mlx5/core/steering/hws/send.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/send.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/send.c
index df3b93499eebb..21e2d0a26e88a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/send.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/send.c
@@ -1005,7 +1005,9 @@ static int hws_send_ring_open_cq(struct mlx5_core_dev *mdev,
 
 static void hws_send_ring_close_cq(struct mlx5hws_send_ring_cq *cq)
 {
-	mlx5_core_destroy_cq(cq->mdev, &cq->mcq);
+	/* A CQ that was not destroyed can still access its buffer, leak it */
+	if (mlx5_core_destroy_cq(cq->mdev, &cq->mcq))
+		return;
 	mlx5_wq_destroy(&cq->wq_ctrl);
 }
 
-- 
2.43.0

Re: [PATCH net-next v1 0/4] net/mlx5: Don't free CQs whose destroy failed

[Junxian Huang]

Hi Yuho,

On 2026/10/16 12:57, Yuho Choi wrote:
> mlx5_core_destroy_cq() can fail, and when it does the CQ may still
> exist in firmware with access to its buffer and doorbell record. It
> also returns early in that case, without waiting for EQ handlers that
> may already hold a reference to the CQ. The four helpers in this series
> ignore that error and free the CQ buffer anyway, while their teardown
> paths also free the object that embeds the struct mlx5_core_cq.
> 
> Keep the buffer and the object that embeds the CQ when DESTROY_CQ

Are the buffer and object freed somewhere else later? Or are they
leaked forever?

Thanks,
Junxian

> fails, as the regular mlx5_ib CQ destroy path does. The HWS send ring
> CQs are polled and never armed, so the HWS patch keeps only their
> buffer rather than passing the error up through the queue open and
> close paths.
> 
> The issues were found with a static analysis tool from ongoing, not yet
> published research, and confirmed by code inspection. None of them was
> reproduced, so this targets net-next; I can resend for net if you
> prefer.
> 
> Compile-tested only (x86_64 defconfig with MLX5_CORE_EN, MLX5_ESWITCH,
> MLX5_SW_STEERING and MLX5_HW_STEERING, W=1).
> 
> Yuho Choi (4):
>   net/mlx5: Don't free the ASO CQ if destroying it fails
>   net/mlx5: Don't free the WC test CQ if destroying it fails
>   net/mlx5: DR, Don't free the send ring CQ if destroying it fails
>   net/mlx5: HWS, Don't free the send ring CQ buffer if destroying the CQ
>     fails
> 
>  .../net/ethernet/mellanox/mlx5/core/lib/aso.c  | 18 +++++++++++++-----
>  .../mellanox/mlx5/core/steering/hws/send.c     |  4 +++-
>  .../mellanox/mlx5/core/steering/sws/dr_send.c  |  4 +++-
>  drivers/net/ethernet/mellanox/mlx5/core/wc.c   | 17 ++++++++++++-----
>  4 files changed, 31 insertions(+), 12 deletions(-)
> 
> 
> base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e