RDMA/mlx5: Don't access ib_qp fields in internal destroy QP path
authorLeon Romanovsky <leonro@mellanox.com>
Wed, 17 Jun 2020 13:01:48 +0000 (16:01 +0300)
committerJason Gunthorpe <jgg@mellanox.com>
Thu, 18 Jun 2020 17:26:04 +0000 (14:26 -0300)
destroy_qp_common is called for flows where QP is already created by
HW. While it is called from IB/core, the ibqp.* fields will be fully
initialized, but it is not the case if this function is called during QP
creation.

Don't rely on ibqp fields as much as possible and initialize
send_cq/recv_cq as temporal solution till all drivers will be converted to
IB/core QP allocation scheme.

refcount_t: underflow; use-after-free.
WARNING: CPU: 1 PID: 5372 at lib/refcount.c:28 refcount_warn_saturate+0xfe/0x1a0
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 5372 Comm: syz-executor.2 Not tainted 5.5.0-rc5 #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
Call Trace:
 mlx5_core_put_rsc+0x70/0x80
 destroy_resource_common+0x8e/0xb0
 mlx5_core_destroy_qp+0xaf/0x1d0
 mlx5_ib_destroy_qp+0xeb0/0x1460
 ib_destroy_qp_user+0x2d5/0x7d0
 create_qp+0xed3/0x2130
 ib_uverbs_create_qp+0x13e/0x190
 ? ib_uverbs_ex_create_qp
 ib_uverbs_write+0xaa5/0xdf0
 __vfs_write+0x7c/0x100
 ksys_write+0xc8/0x200
 do_syscall_64+0x9c/0x390
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: 08d53976609a ("RDMA/mlx5: Copy response to the user in one place")
Link: https://lore.kernel.org/r/20200617130148.2846643-1-leon@kernel.org
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
drivers/infiniband/hw/mlx5/qp.c

index dbe82cdb8d2c5e8d2fee0a209cf5277f5063a444..1e567fe3a527e38949ba5c6ac141a0fd17b62df6 100644 (file)
@@ -2341,18 +2341,18 @@ static void destroy_qp_common(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
        unsigned long flags;
        int err;
 
-       if (qp->ibqp.rwq_ind_tbl) {
+       if (qp->is_rss) {
                destroy_rss_raw_qp_tir(dev, qp);
                return;
        }
 
-       base = (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
+       base = (qp->type == IB_QPT_RAW_PACKET ||
                qp->flags & IB_QP_CREATE_SOURCE_QPN) ?
-              &qp->raw_packet_qp.rq.base :
-              &qp->trans_qp.base;
+                      &qp->raw_packet_qp.rq.base :
+                      &qp->trans_qp.base;
 
        if (qp->state != IB_QPS_RESET) {
-               if (qp->ibqp.qp_type != IB_QPT_RAW_PACKET &&
+               if (qp->type != IB_QPT_RAW_PACKET &&
                    !(qp->flags & IB_QP_CREATE_SOURCE_QPN)) {
                        err = mlx5_core_qp_modify(dev, MLX5_CMD_OP_2RST_QP, 0,
                                                  NULL, &base->mqp, NULL);
@@ -2368,8 +2368,8 @@ static void destroy_qp_common(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
                                     base->mqp.qpn);
        }
 
-       get_cqs(qp->ibqp.qp_type, qp->ibqp.send_cq, qp->ibqp.recv_cq,
-               &send_cq, &recv_cq);
+       get_cqs(qp->type, qp->ibqp.send_cq, qp->ibqp.recv_cq, &send_cq,
+               &recv_cq);
 
        spin_lock_irqsave(&dev->reset_flow_resource_lock, flags);
        mlx5_ib_lock_cqs(send_cq, recv_cq);
@@ -2391,7 +2391,7 @@ static void destroy_qp_common(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
        mlx5_ib_unlock_cqs(send_cq, recv_cq);
        spin_unlock_irqrestore(&dev->reset_flow_resource_lock, flags);
 
-       if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
+       if (qp->type == IB_QPT_RAW_PACKET ||
            qp->flags & IB_QP_CREATE_SOURCE_QPN) {
                destroy_raw_packet_qp(dev, qp);
        } else {
@@ -3002,10 +3002,18 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attr,
        return &qp->ibqp;
 
 destroy_qp:
-       if (qp->type == MLX5_IB_QPT_DCT)
+       if (qp->type == MLX5_IB_QPT_DCT) {
                mlx5_ib_destroy_dct(qp);
-       else
+       } else {
+               /*
+                * The two lines below are temp solution till QP allocation
+                * will be moved to be under IB/core responsiblity.
+                */
+               qp->ibqp.send_cq = attr->send_cq;
+               qp->ibqp.recv_cq = attr->recv_cq;
                destroy_qp_common(dev, qp, udata);
+       }
+
        qp = NULL;
 free_qp:
        kfree(qp);