Re: [PATCH rdma-next 10/10] RDMA/mana_ib: Drain kernel receive and send queues

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rdma/mana_ib: null pointer dereference in mana_ib_drain_sq() on UD/GSI QPs
- [Medium] rdma/mana_ib: user-triggerable WARN_ONCE via manual comp_handler invocation for direct CQs
--

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

RDMA/mana_ib: Drain kernel receive and send queues

This commit implements the core drain callbacks for the mana_ib driver
by moving the QP to the ERR state, posting a completion-bearing marker,
and waiting until that marker is flushed.

> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 0f60909cbe848..4e6f15fb4887b 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c

[ ... ]

> +void mana_ib_drain_sq(struct ib_qp *ibqp)
> +{
> +	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
> +	struct mana_ib_drain_cqe sdrain;
> +	struct ib_rdma_wr swr = {
> +		.wr = {
> +			.next = NULL,
> +			.wr_cqe = &sdrain.cqe,
> +			.opcode = IB_WR_SEND,
> +		},
> +	};

[Severity: High]
Does this WR structure correctly handle IB_QPT_UD or IB_QPT_GSI queue types?

When mana_ib_drain_sq() is called on a UD QP, it submits this stack-allocated
ib_rdma_wr structure to ib_post_send(). The mana driver's send path in
mana_ib_post_send_ud_one() expects an ib_ud_wr structure and downcasts this
generic WR into it.

Because the ib_rdma_wr structure is smaller than ib_ud_wr, the downcast
results in an out-of-bounds stack read over the overlapping memory. This
causes the evaluated ah pointer to be NULL.

When the hardware scatter-gather list is built later via
gdma_sgl[0].address = ah->dma_handle, dereferencing the NULL ah pointer
causes a kernel panic.

Since unprivileged userspace applications can create and drain UD QPs via
uverbs, can this be exploited as a local denial-of-service vulnerability?

[ ... ]

> +	ret = ib_post_send(ibqp, &swr.wr, &bad_swr);
> +	if (ret) {
> +		ibdev_err(ibqp->device, "Failed to post SQ drain WR: %d\n", ret);
> +		return;
> +	}
> +
> +	if (cq->comp_handler)
> +		cq->comp_handler(cq, cq->cq_context);

[Severity: Medium]
Is it safe to execute the comp_handler here unconditionally before checking
if the CQ uses direct polling?

For CQs instantiated with IB_POLL_DIRECT, the RDMA core strictly assigns
ib_cq_completion_direct() to the comp_handler to emit a WARN_ONCE on any
unsolicited execution.

Unprivileged users can trigger QP draining via uverbs on direct polling CQs,
meaning they can trivially trip this warning.

If the system is configured with panic_on_warn, could this lead to an
unexpected denial of service?

> +
> +	if (cq->poll_ctx == IB_POLL_DIRECT) {

[ ... ]

> +void mana_ib_drain_rq(struct ib_qp *ibqp)
> +{

[ ... ]

> +	ret = ib_post_recv(ibqp, &rwr, NULL);
> +	if (ret) {
> +		ibdev_err(ibqp->device, "Failed to post RQ drain WR: %d\n", ret);
> +		return;
> +	}
> +
> +	if (cq->comp_handler)
> +		cq->comp_handler(cq, cq->cq_context);

[Severity: Medium]
Similarly to the send queue drain, will this unconditional comp_handler
execution trigger the core's unsolicited completion warning for direct CQs?

> +
> +	if (cq->poll_ctx == IB_POLL_DIRECT) {

[ ... ]

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