Re: [PATCH rdma-next v2 08/10] RDMA/mana_ib: Flush and notify CQs when kernel QPs enter ERR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/mana_ib: Use-after-free of UC QPs on CQ error lists
--

commit c99026ba35e2170c2d6c2bf5a5f090a5f44a790e
Author: Konstantin Taranov <kotaranov@microsoft.com>

RDMA/mana_ib: Flush and notify CQs when kernel QPs enter ERR

This commit completes the RC software flush path by adding a kernel QP
to the CQ error lists when it transitions to the ERR state. It also invokes
the CQ handlers after the transition to flush pending work requests.

> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 74cf80074e54f..1a1764d51552e 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
[ ... ]
> @@ -1028,15 +1046,16 @@ static int mana_ib_gd_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>  	return 0;
>  }
>  
> -static void mana_ib_modify_qp_state(struct ib_qp *ibqp, struct ib_qp_attr *attr,
> +static bool mana_ib_modify_qp_state(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>  				    int attr_mask, struct ib_udata *udata)
>  {
>  	struct mana_ib_dev *mdev = container_of(ibqp->device, struct mana_ib_dev, ib_dev);
>  	struct mana_ib_qp *qp = container_of(ibqp, struct mana_ib_qp, ibqp);
>  	struct gdma_queue *rq;
> +	bool notify = false;
>  
>  	if (udata)
> -		return;
> +		return false;
>  
>  	if (attr_mask & IB_QP_PATH_MTU)
>  		qp->mtu = attr->path_mtu;
> @@ -1049,6 +1068,11 @@ static void mana_ib_modify_qp_state(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>  		default:
>  			break;
>  		}
> +		qp->state = attr->qp_state;
> +		if (attr->qp_state == IB_QPS_ERR) {
> +			mana_add_qp_to_error_cqs(qp);
> +			notify = true;
> +		}
>  	}

[Severity: High]
Can this inadvertently link user-created UC QPs into the CQ's error lists,
leading to a use-after-free when the QP is destroyed?

Although UC QPs are created by user space, RDMA Connection Manager (CM)
manages connection teardowns and errors from the kernel. When an error
occurs or the connection is aborted, CM calls ib_modify_qp() via
cma_modify_qp_err() with udata = NULL.

This bypasses the earlier udata check and mistakenly adds the UC QP to
the CQ's error lists.

Would it make sense to update mana_ib_destroy_uc_qp() to call
mana_remove_qp_from_cqs(), similar to how RC, UD, and GSI QPs are handled?
Currently, it omits this cleanup:

drivers/infiniband/hw/mana/qp.c:mana_ib_destroy_uc_qp() {
    ...
	mana_table_remove_qp(mdev, qp);
	/* Ignore return code as there is not much we can do about it.
	 * The error message is printed inside.
	 */
	mana_ib_gd_destroy_rnic_qp(mdev, qp);
	for (i = 0; i < MANA_UC_QUEUE_TYPE_MAX; ++i)
		mana_ib_destroy_queue(mdev, &qp->uc_qp.queues[i]);
    ...
}

Because of this omission, the RDMA core frees the QP memory while the freed
QP's list nodes remain linked in the CQ's error lists.

A subsequent CQ poll or flush event will iterate over the freed QP, resulting
in a use-after-free read or list corruption.

Additionally, since UC QPs do not allocate shadow queues, attempting to flush
them in mana_flush_completions() will cause a division-by-zero
(queue->consumer % queue->size) because queue->size is 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003064037.2092669-1-kotaranov@linux.microsoft.com?part=8