Re: [PATCH for-rc v2 3/8] RDMA/bnxt_re: Validate num_sge in bnxt_re_post_srq_recv()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Unconditional error logging on fast-path enables unprivileged log spam (Denial of Service)

Pre-existing issues:
- [High] Uninitialized stack memory in hardware work queue descriptor (wqe.flags)
--

commit a50a4d21bcd2c8e40025a33fa29a7f621d05a408
Author: Selvin Xavier <selvin.xavier@broadcom.com>

RDMA/bnxt_re: Validate num_sge in bnxt_re_post_srq_recv()

This commit adds a validation check in bnxt_re_post_srq_recv() to ensure
that the number of scatter/gather elements (SGE) provided for a Shared
Receive Queue (SRQ) does not exceed the device's configured maximum.

> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index e56830a669bf4..ab9d4d6bf5b02 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -2386,9 +2386,16 @@ int bnxt_re_post_srq_recv(struct ib_srq *ib_srq, const struct ib_recv_wr *wr,
>  			  const struct ib_recv_wr **bad_wr)
>  {
>  	struct bnxt_re_srq *srq = container_of(ib_srq, struct bnxt_re_srq,
>  					       ib_srq);
>  	struct bnxt_qplib_swqe wqe;

[Severity: High]
This is a pre-existing issue, but does this allocation leave wqe.flags
uninitialized before the structure is passed to the hardware?

Any legitimate call to bnxt_re_post_srq_recv allocates the wqe structure on
the stack without zeroing it. The function sets specific fields but appears
to leave wqe.flags uninitialized before calling bnxt_qplib_post_srq_recv,
which writes the flags field into the hardware's ring buffer descriptor.

Can this result in unpredictable hardware behavior or dropped work requests
if the garbage stack data is misinterpreted by the hardware datapath?

>  	unsigned long flags;
>  	int rc = 0;
>  
>  	spin_lock_irqsave(&srq->lock, flags);
>  	while (wr) {
>  		/* Transcribe each ib_recv_wr to qplib_swqe */
>  		wqe.num_sge = wr->num_sge;
> +		if (wr->num_sge > srq->qplib_srq.max_sge) {
> +			ibdev_err(&srq->rdev->ibdev,
> +				  "Limit exceeded for SRQ Receive SGEs");

[Severity: Medium]
Since this function is on a fast-path datapath reachable from unprivileged
userspace via the RDMA uverbs API, could this unconditional error log lead
to log spam?

An unprivileged userspace application could intentionally flood the kernel
ring buffer by passing a wr->num_sge that is valid for the system call but
exceeds the driver's max_sge limit. 

Would it be better to rate-limit this ibdev_err call, or remove it entirely
to prevent potential denial of service through disk space exhaustion and
system performance degradation?

> +			rc = -EINVAL;
> +			*bad_wr = wr;
> +			break;
> +		}
>  		bnxt_re_build_sgl(wr->sg_list, wqe.sg_list, wr->num_sge);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909135244.122747-1-selvin.xavier@broadcom.com?part=3