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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mana: Missing bounds checks on QP capacities cause integer overflow and undefined behavior
- [Medium] mana: Integer underflow in mana_ib_query_device when SGE count is 0
--

commit 5b497775948fa714374d6418c365f7b62d67deb5
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/main.c b/drivers/infiniband/hw/mana/main.c
> index fa47c988a2747..329bc9f30aa10 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -637,7 +637,8 @@ int mana_ib_query_device(struct ib_device *ibdev, struct ib_device_attr *props,
>  	props->max_qp = dev->adapter_caps.max_qp_count;
>  	props->max_qp_wr = dev->adapter_caps.max_qp_wr;
>  	props->device_cap_flags = IB_DEVICE_RC_RNR_NAK_GEN;
> -	props->max_send_sge = dev->adapter_caps.max_send_sge_count;
> +	/* Subtract 1 from max_send_sge to account for the reserved SGE */
> +	props->max_send_sge = dev->adapter_caps.max_send_sge_count - 1;

[Severity: Medium]
Could this subtraction underflow if the firmware reports max_send_sge_count
as 0?

If it underflows to 0xffffffff, this massively inflated value would be
reported to the RDMA core, which relies on it to bound user QP requests.
Could this allow malicious userspace to bypass sizing limits and induce
integer overflows during queue allocations?

>  	props->max_recv_sge = dev->adapter_caps.max_recv_sge_count;
>  	props->max_sge_rd = dev->adapter_caps.max_recv_sge_count;
>  	props->max_cq = dev->adapter_caps.max_cq_count;

[ ... ]

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

[Severity: High]
Does this missing bounds check on attr->cap allow for integer overflow or
undefined behavior here?

In mana_ib_create_rc_qp_kernel() and mana_ib_create_ud_qp(), attr->cap is
passed to mana_ib_queue_size() without bounds-checking against the adapter's
capabilities, unlike mana_ib_create_qp_raw() which validates the queue
capacities.

If max_send_wr and max_send_sge are abnormally large, could this 32-bit u32
multiplication overflow and wrap around to a small queue_size? 

This would lead to an undersized GDMA queue allocation while the massive
size is sent to the RNIC, potentially resulting in out-of-bounds DMA access.

Additionally, if QPs are legitimately created with max_send_wr = 0 or
max_recv_wr = 0 (e.g., receive-only), queue_size becomes 0. The subsequent
call to roundup_pow_of_two(0) at the end of mana_ib_queue_size() evaluates
to fls_long(-1), which is undefined behavior in the kernel.

> +			break;
> +		case MANA_RC_SEND_QUEUE_MMQ:
> +			queue_size = attr->cap.max_send_wr *
> +				mana_ib_wqe_size(1U, INLINE_OOB_EXTRA_LARGE_SIZE);
> +			break;
> +		case MANA_RC_SEND_QUEUE_RESPONDER:
> +			queue_size = MANA_PAGE_SIZE;
> +			break;

[ ... ]

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