Re: [PATCH rdma-next v4 02/10] RDMA/mana: Create and destroy kernel RC QPs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rdma/mana: integer overflow and undefined behavior in QP queue size allocation
--

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

    RDMA/mana: Create and destroy kernel RC QPs
    
    Implement kernel RC QP creation and destruction. This feature requires
    extended-WQE and power-of-two-SQ support from the HW.

> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 15c5d6cefd628..edaf03372cd81 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c

[ ... ]

> @@ -442,6 +442,32 @@ static u32 mana_ib_queue_size(struct ib_qp_init_attr *attr, u32 queue_type)
>  			queue_size = attr->cap.max_recv_wr *
>  				mana_ib_wqe_size(attr->cap.max_recv_sge, INLINE_OOB_SMALL_SIZE);
>  		break;
> +	case IB_QPT_RC:
> +		switch (queue_type) {
> +		case MANA_RC_SEND_QUEUE_REQUESTER:
> +			queue_size = attr->cap.max_send_wr *
> +				mana_ib_fixed_wqe_size(attr->cap.max_send_sge,
> +						       INLINE_OOB_EXTRA_LARGE_SIZE);
> +			break;

[Severity: High]
Can this 32-bit multiplication overflow if max_send_wr and max_send_sge are
unvalidated? The kernel ULP can provide an arbitrarily large max_send_wr and
max_recv_wr via attr->cap.

If attr->cap.max_send_wr is 0, queue_size will evaluate to 0. Since
mana_ib_queue_size() later returns
MANA_PAGE_ALIGN(roundup_pow_of_two(queue_size)), doesn't
roundup_pow_of_two(0) result in undefined behavior?

[ ... ]

> +static int mana_ib_create_rc_qp_kernel(struct ib_qp *ibqp, struct ib_pd *ibpd,
> +				       struct ib_qp_init_attr *attr)
> +{
> +	struct mana_ib_dev *mdev = container_of(ibpd->device, struct mana_ib_dev, ib_dev);
> +	struct mana_ib_qp *qp = container_of(ibqp, struct mana_ib_qp, ibqp);
> +	u32 doorbell = mdev->gdma_dev->doorbell;
> +	u32 queue_size;
> +	u64 flags = 0;
> +	int i, err;
> +
> +	if (!(mdev->adapter_caps.feature_flags & MANA_IB_FEATURE_EXTENDED_WQE_FORMAT))
> +		return -EOPNOTSUPP;
> +	if (!(mdev->adapter_caps.feature_flags & MANA_IB_FEATURE_RC_QP_SQ_POW2_SUPPORT))
> +		return -EOPNOTSUPP;
> +
> +	for (i = 0; i < MANA_RC_QUEUE_TYPE_MAX; ++i) {
> +		queue_size = mana_ib_queue_size(attr, i);
> +		err = mana_ib_create_kernel_queue(mdev, queue_size,

[Severity: High]
Does this code blindly pass the potentially overflowed queue_size to hardware
queue allocation without prior validation of attr->cap limits against
mdev->adapter_caps.max_qp_wr? The hardware could overrun an undersized DMA
queue, leading to memory corruption.

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