[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

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

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016045849.813696-6-oss.patchbox@gmail.com?part=1

[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/mlx5: SQ DMA use-after-free due to dropped DESTROY_SQ error
--

commit 7355666e6a6920857efdc4ded19eec645c9bd315
Author: Yuho Choi <oss.patchbox@gmail.com>

net/mlx5: Don't free the WC test CQ if destroying it fails

This commit fixes a DMA use-after-free by returning the error from
mlx5_wc_destroy_cq() and keeping the CQ buffer and struct mlx5_wc_sq
if DESTROY_CQ fails. This prevents the hardware from retaining access
to a freed buffer.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/wc.c b/drivers/net/ethernet/mellanox/mlx5/core/wc.c
> index 7d3d4d739e2cd..01e9800ff6fd9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/wc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/wc.c

[ ... ]

> @@ -396,11 +401,13 @@ static void mlx5_core_test_wc(struct mlx5_core_dev *mdev)
>  	mlx5_wc_destroy_sq(sq);

[Severity: High]
Does this same vulnerability still exist for the SQ?

If the firmware DESTROY_SQ command fails (e.g., due to a timeout), the
hardware retains access to the SQ DMA buffer. Because
mlx5_core_destroy_sq() drops the error and returns void:

drivers/net/ethernet/mellanox/mlx5/core/transobj.c:mlx5_core_destroy_sq() {
    ...
    mlx5_cmd_exec_in(dev, destroy_sq, in);
}

The caller unconditionally frees the SQ buffer via mlx5_wq_destroy():

drivers/net/ethernet/mellanox/mlx5/core/wc.c:mlx5_wc_destroy_sq() {
    mlx5_core_destroy_sq(sq->cq.mdev, sq->sqn);
    mlx5_wq_destroy(&sq->wq_ctrl);
}

Could the hardware read garbage data from the freed memory as WQEs,
leading to arbitrary hardware DMA operations, similar to the CQ
vulnerability this patch fixes?

>  
>  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);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016045849.813696-6-oss.patchbox@gmail.com?part=2

[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

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

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016045849.813696-6-oss.patchbox@gmail.com?part=3

[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 4/4] net/mlx5: HWS, Don't free the send ring CQ buffer if destroying the CQ fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mlx5: software UAF of embedded CQ struct when firmware destroy command fails
--

commit 46458ea83b82e1a1672d9e95c03b0f4bac750b05
Author: Yuho Choi <oss.patchbox@gmail.com>

net/mlx5: HWS, Don't free the send ring CQ buffer if destroying the CQ fails

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.

> 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;

[Severity: High]
Does returning early here expose the software CQ structure to a use-after-free?

When mlx5_core_destroy_cq() fails, it skips synchronization and completion
wait for concurrent users of the software CQ:

drivers/net/ethernet/mellanox/mlx5/core/cq.c:mlx5_core_destroy_cq() {
	...
	err = mlx5_cmd_exec_in(dev, destroy_cq, in);
	if (err)
		return err;

	synchronize_irq(cq->irqn);
	mlx5_cq_put(cq);
	wait_for_completion(&cq->free);
	...
}

Because hws_send_ring_close_cq() now returns without any indication of failure,
its caller (mlx5hws_send_queues_close()) will proceed to unconditionally free
the array embedding the CQ structure:

drivers/net/ethernet/mellanox/mlx5/core/steering/hws/send.c:mlx5hws_send_queues_close() {
	...
	kfree(ctx->send_queue);
	...
}

Since wait_for_completion() and synchronize_irq() were skipped, could an
executing concurrent EQ interrupt handler or async event handler still hold
a reference to mcq and access it after the send_queue array has been freed?

>  	mlx5_wq_destroy(&cq->wq_ctrl);
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016045849.813696-6-oss.patchbox@gmail.com?part=4

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

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

[Yuho Choi]

On Fri, 16 Oct 2026 at 02:59, Junxian Huang <huangjunxian6@hisilicon.com> wrote:
>
> Are the buffer and object freed somewhere else later? Or are they
> leaked forever?

They are leaked; nothing frees them later. This only happens when the
firmware is alive but rejects DESTROY_CQ. A device in internal error
still frees everything as before.

Freeing them later would mean tracking them until the device is torn
down, which seemed like too much for this failure case. I can look into
reclaiming them if you think it is worth it.

Thanks,
Yuho