Re: [PATCH rdma-next v3 10/13] RDMA/core: queue pair attributes for unconnected transports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] RDMA/core: incomplete zero-size ordering check in
  create_qp_opt_attrs()
--

commit ab78e9acb0c6a04e98901cf4dece43df9ce37611
Author: Eric Davis <eric.davis@broadcom.com>

RDMA/core: queue pair attributes for unconnected transports

This commit adds creation attributes for reliable unconnected (IB_QPT_RU)
queue pairs, including initiator source ID, job key binding, and ordering
semantics validation.

> diff --git a/drivers/infiniband/core/uverbs_std_types_qp.c b/drivers/infiniband/core/uverbs_std_types_qp.c
> index 249206d2992f9..a1112083fb73f 100644
> --- a/drivers/infiniband/core/uverbs_std_types_qp.c
> +++ b/drivers/infiniband/core/uverbs_std_types_qp.c

[ ... ]

> +	if (has_semantics) {
> +		ret = uverbs_copy_from_or_zero(&usem, attrs,
> +					       UVERBS_ATTR_CREATE_QP_SEMANTICS);
> +		if (ret)
> +			return ret;
> +
> +		m = usem.comp_mask;
> +		if ((m & ~QP_SEMANTICS_MASK_ALL) ||
> +		    ((m & IB_UVERBS_QP_SEMANTICS_MASK_MSG_ORDER) &&
> +		     (usem.msg_order & ~QP_ORDER_ALL)) ||
> +		    ((m & IB_UVERBS_QP_SEMANTICS_MASK_USAGE) &&
> +		     (usem.usage_flags & ~QP_USAGE_ALL)) ||
> +		    memchr_inv(usem.reserved, 0, sizeof(usem.reserved)))
> +			return -EINVAL;
> +
> +		/* only the fields comp_mask selects reach the driver */
> +		sem->comp_mask = m;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_MSG_ORDER)
> +			sem->msg_order = usem.msg_order;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_RAW)
> +			sem->max_rdma_raw_size = usem.max_rdma_raw_size;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_WAR)
> +			sem->max_rdma_war_size = usem.max_rdma_war_size;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_WAW)
> +			sem->max_rdma_waw_size = usem.max_rdma_waw_size;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_PDU)
> +			sem->max_pdu = usem.max_pdu;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_USAGE)
> +			sem->usage_flags = usem.usage_flags;
> +		if (m & IB_UVERBS_QP_SEMANTICS_MASK_IMM)
> +			sem->imm_data_size = usem.imm_data_size;
> +
> +		/* nor an ordering whose data-ordering size it gives as 0 */
> +		if (((m & IB_UVERBS_QP_SEMANTICS_MASK_RAW) &&
> +		     !sem->max_rdma_raw_size &&
> +		     (sem->msg_order & QP_ORDER_RAW_SIZED)) ||
> +		    ((m & IB_UVERBS_QP_SEMANTICS_MASK_WAR) &&
> +		     !sem->max_rdma_war_size &&
> +		     (sem->msg_order & QP_ORDER_WAR_SIZED)) ||
> +		    ((m & IB_UVERBS_QP_SEMANTICS_MASK_WAW) &&
> +		     !sem->max_rdma_waw_size &&
> +		     (sem->msg_order & QP_ORDER_WAW_SIZED)))
> +			return -EINVAL;

[Severity: Medium]
Can userspace bypass this zero-size ordering check by omitting
IB_UVERBS_QP_SEMANTICS_MASK_MSG_ORDER from comp_mask?

sem->msg_order is only populated when IB_UVERBS_QP_SEMANTICS_MASK_MSG_ORDER
is set in m, remaining 0 otherwise.  If a caller sets
IB_UVERBS_QP_SEMANTICS_MASK_RAW with max_rdma_raw_size == 0 and leaves
IB_UVERBS_QP_SEMANTICS_MASK_MSG_ORDER clear, sem->msg_order is 0 and this
check passes even though the caller did not relax msg_order and the device
default adv.msg_order still has QP_ORDER_RAW_SIZED set, passing inconsistent
semantics in attr->qp_semantics to the driver.  Should this check test
against the effective msg_order (using adv.msg_order when
IB_UVERBS_QP_SEMANTICS_MASK_MSG_ORDER is not set, and conversely checking
adv.max_rdma_*_size when a sized ordering bit is requested without its size
mask)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-uet-ib-core-v3-0-3e7e7ffed2e9@broadcom.com?part=10