Re: [PATCH rdma-next 14/23] RDMA/mlx5: Block generic CQ poll path in adversarial mode

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message and the new comment in mlx5_poll_one()
  (drivers/infiniband/hw/mlx5/cq.c) give the wrong example of the…

--

RDMA/mlx5: Block generic CQ poll path in adversarial mode

This commit makes mlx5_poll_one() return -EOPNOTSUPP when
mlx5_ib_adversarial_mode is set. The generic CQ poller trusts CQE
content, so with this change it never interprets a CQE from an untrusted
NIC. The UMR CQ is expected to use its own validating poll_one instead.

> diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> index 1a98263ec3111..5cc86c3f93f3e 100644
> --- a/drivers/infiniband/hw/mlx5/cq.c
> +++ b/drivers/infiniband/hw/mlx5/cq.c
> @@ -464,6 +464,15 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
>  	void *cqe;
>  	int idx;
>  
> +	/*
> +	 * This path trusts the CQE, so refuse it in adversarial_mode. Only an
> +	 * unexpected caller gets here, e.g. the uverbs POLL_CQ command or an
> +	 * event on the UMR CQ before its own poll_one is installed. Nothing is
> +	 * consumed, so such a CQE is left for that poll_one.
> +	 */

[Severity: Low]
Can the uverbs POLL_CQ command actually reach mlx5_poll_one() on mlx5?
The commit message uses the same example:

  "Refuse it there, so an unexpected caller, such as the uverbs POLL_CQ
  command, finds the CQ empty and no CQE is interpreted."

_ib_alloc_device() builds the default uverbs_cmd_mask without
IB_USER_VERBS_CMD_POLL_CQ. mlx5 never adds that bit; only bnxt_re and
rdmavt set it. As a result, uapi_create_write() disables the write
method:

drivers/infiniband/core/uverbs_uapi.c:uapi_create_write() {
    ...
	method_elm->disabled = !(ibdev->uverbs_cmd_mask &
				 BIT_ULL(def->write.command_num));
    ...
}

uapi_finalize_disable() then removes the method from the uapi tree. So
POLL_CQ doesn't appear to be reachable on mlx5.

The case that can be reached seems to be the UMR CQ during setup.
mlx5r_umr_resource_init() arms the CQ here:

	cq = ib_alloc_cq(&dev->ib_dev, NULL, 128, 0, IB_POLL_SOFTIRQ);

mlx5_ib_create_cq() sets cq->poll_one = mlx5_poll_one.
mlx5_ib_poll_one_umr is only installed later, after ib_create_qp() and
mlx5r_umr_qp_rst2rts().

If a completion event with a forged CQE arrives in that window, the
softirq poller calls mlx5_poll_one(). The earlier "Verify UMR CQ polling
against a hostile NIC" patch leaves this window open, and it looks like
the case this patch really closes.

The code comment mentions this window, but the commit message doesn't.
Could the commit message describe the UMR setup window as the case being
fixed? Could both the commit message and the comment drop the POLL_CQ
example?

The commit message also says:

  "kernel ULPs don't bind to the device"

ib_device::adversarial_mode is only set by the later "RDMA/mlx5:
Advertise adversarial mode via ib_device::adversarial_mode" patch. Is
that statement accurate at this commit?

> +	if (unlikely(mlx5_ib_adversarial_mode))
> +		return -EOPNOTSUPP;
> +
>  repoll:
>  	cqe = next_cqe_sw(cq);
>  	if (!cqe)

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103163949.1554-1-yishaih%40nvidia.com