[PATCH v2] vdpa/mlx5: Don't free QP buffers if destroying the QP fails

[PATCH v2] vdpa/mlx5: Don't free QP buffers if destroying the QP fails

[Yuho Choi]

qp_destroy() frees the RQ buffer and doorbell record of a driver-owned
QP even when DESTROY_QP fails. The QP then still exists in firmware and
can read WQEs and the doorbell record from freed memory.

Return without freeing them in that case, as cq_destroy() does, and
include the QP type and the error in the warning.

Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
v2:
- Shorten the commit message (Dragos).
- Drop the Fixes and Cc: stable tags (Dragos).
- Print the QP type and the error in the warning (Dragos).
- Rebased on v7.4-rc1.

Compile-tested with W=1.

 drivers/vdpa/mlx5/net/mlx5_vnet.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 8563fec2855d..20f4ff67c975 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -479,12 +479,18 @@ static int qp_create(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mvq
 static void qp_destroy(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_qp *vqp)
 {
 	u32 in[MLX5_ST_SZ_DW(destroy_qp_in)] = {};
+	int err;
 
 	MLX5_SET(destroy_qp_in, in, opcode, MLX5_CMD_OP_DESTROY_QP);
 	MLX5_SET(destroy_qp_in, in, qpn, vqp->mqp.qpn);
 	MLX5_SET(destroy_qp_in, in, uid, ndev->mvdev.res.uid);
-	if (mlx5_cmd_exec_in(ndev->mvdev.mdev, destroy_qp, in))
-		mlx5_vdpa_warn(&ndev->mvdev, "destroy qp 0x%x\n", vqp->mqp.qpn);
+	err = mlx5_cmd_exec_in(ndev->mvdev.mdev, destroy_qp, in);
+	if (err) {
+		/* The QP still owns its buffers */
+		mlx5_vdpa_warn(&ndev->mvdev, "destroy %s qp 0x%x failed: %d\n",
+			       vqp->fw ? "fw" : "sw", vqp->mqp.qpn, err);
+		return;
+	}
 	if (!vqp->fw) {
 		mlx5_db_free(ndev->mvdev.mdev, &vqp->db);
 		rq_buf_free(ndev, vqp);
-- 
2.43.0

Re: [PATCH v2] vdpa/mlx5: Don't free QP buffers if destroying the QP fails

[sashiko-bot]

> qp_destroy() frees the RQ buffer and doorbell record of a driver-owned
> QP even when DESTROY_QP fails. The QP then still exists in firmware and
> can read WQEs and the doorbell record from freed memory.
> 
> Return without freeing them in that case, as cq_destroy() does, and
> include the QP type and the error in the warning.
> 
> 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/20261102204734.443140-2-oss.patchbox@gmail.com?part=1