Re: [PATCH rdma-next 03/10] net/mana: Extend GDMA encoding for new RDMA WQEs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rdma/mana: Missing fixed WQE size validation causes ring buffer overflow
--

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

net/mana: Extend GDMA encoding for new RDMA WQEs

This commit extends mana_gd_post_work_request() to support extra-large OOB
data and fixed-size WQEs. It encodes WQE header fields based on WR flags
and uses num_pad_sge to specify padding size for fixed-size WQEs.

> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 05ba7055405da..444101dcbb647 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1699,8 +1699,27 @@ static u32 mana_gd_write_client_oob(const struct gdma_wqe_request *wqe_req,
>  	header->num_sge = wqe_req->num_sge;
>  	header->inline_oob_size_div4 = client_oob_size / sizeof(u32);
>  
> +	if (client_oob_size == INLINE_OOB_EXTRA_LARGE_SIZE) {
> +		/* Extra large oob is encoded as 7 */
> +		header->inline_oob_size_div4 = 7;
> +		/* Extra large oob has 8 byte padding  */
> +		client_oob_size += 8;
> +	}
> +
> +	if (wqe_req->wqe_size_in_bu) {
> +		header->num_sge = (wqe_req->wqe_size_in_bu * GDMA_WQE_BU_SIZE
> +			- sizeof(struct gdma_wqe) - client_oob_size) / sizeof(struct gdma_sge);
> +		header->num_pad_sge = header->num_sge - wqe_req->num_sge;

[Severity: High]
Does this calculation risk silently underflowing if the fixed WQE size is too
small?

If a caller provides an undersized fixed WQE size, this subtraction could
underflow, which might corrupt the hardware descriptor bitfields and lead to an
out-of-bounds write of SGEs.

> +	}
> +
> +	header->check_rsn = !!(wqe_req->flags & GDMA_WR_CHECK_RSN);

[ ... ]

> @@ -1792,15 +1814,26 @@ int mana_gd_post_work_request(struct gdma_queue *wq,
>  		max_wqe_size = GDMA_MAX_RQE_SIZE;
>  	} else {
>  		if (client_oob_size != INLINE_OOB_SMALL_SIZE &&
> -		    client_oob_size != INLINE_OOB_LARGE_SIZE)
> +		    client_oob_size != INLINE_OOB_LARGE_SIZE &&
> +		    client_oob_size != INLINE_OOB_EXTRA_LARGE_SIZE)
>  			return -EINVAL;
>  
>  		max_wqe_size = GDMA_MAX_SQE_SIZE;
>  	}
>  
>  	sgl_data_size = sizeof(struct gdma_sge) * wqe_req->num_sge;
> -	wqe_size = ALIGN(sizeof(struct gdma_wqe) + client_oob_size +
> -			 sgl_data_size, GDMA_WQE_BU_SIZE);
> +	if (wqe_req->wqe_size_in_bu)
> +		wqe_size = wqe_req->wqe_size_in_bu * GDMA_WQE_BU_SIZE;

[Severity: High]
Is it possible for the user-provided fixed WQE size to be accepted here
without verifying that it is actually large enough to contain the requested
SGEs and OOB payload?

If a caller creates a queue pair with a small max_send_sge (fixing the WQE
size) but posts a send work request with num_sge larger than max_send_sge,
could this bypass validation?

If this validation is missing, it appears it would cause the integer underflow
mentioned above in mana_gd_write_client_oob() and lead to an out-of-bounds
memory write during mana_gd_write_sgl().

> +	else
> +		wqe_size = ALIGN(sizeof(struct gdma_wqe) + client_oob_size +
> +				 sgl_data_size, GDMA_WQE_BU_SIZE);

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