Re: [PATCH rdma-next 04/13] RDMA/core: add the reliable unconnected queue pair type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rdma: missing state transitions for IB_QPT_RU break QP modification
- [Medium] rdma: missing IB_QPT_RU validation in legacy ib_uverbs_post_send
--

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

RDMA/core: add the reliable unconnected queue pair type

This commit adds the IB_QPT_RU type and its userspace counterpart. It also
defines the field layout for a reliable unconnected queue pair number used by
UET, and exempts its full 32-bit width from the usual 24-bit restrack
assertion.

[ ... ]

> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 02f3312c7cf24..8d74ed4fc581f 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -1188,6 +1188,7 @@ enum ib_qp_type {
>  	IB_QPT_RAW_PACKET = IB_UVERBS_QPT_RAW_PACKET,
>  	IB_QPT_XRC_INI = IB_UVERBS_QPT_XRC_INI,
>  	IB_QPT_XRC_TGT = IB_UVERBS_QPT_XRC_TGT,
> +	IB_QPT_RU = IB_UVERBS_QPT_RU,

[Severity: High]
With the addition of IB_QPT_RU, should it also be added to qp_state_table[]
in drivers/infiniband/core/verbs.c?

Currently, because IB_QPT_RU is omitted from qp_state_table[], its
configuration logic zero-evaluates. This causes the core helper
ib_modify_qp_is_ok() to block all state advancements for IB_QPT_RU QPs, as
the implicit 0 values flag any transition parameters as invalid:

drivers/infiniband/core/verbs.c:ib_modify_qp_is_ok() {
    ...
    req_param = qp_state_table[cur_state][next_state].req_param[type];
    opt_param = qp_state_table[cur_state][next_state].opt_param[type];

    if ((mask & req_param) != req_param)
        return false;
    ...
}

For example, when transitioning from IB_QPS_RESET to IB_QPS_RTS,
ib_modify_qp_is_ok() checks qp_state_table[IB_QPS_RESET][IB_QPS_RTS].valid,
reads 0, and returns false. This causes the driver to reject the configuration
with -EINVAL.

[Severity: Medium]
Since IB_QPT_RU is an unconnected queue pair type, should
ib_uverbs_post_send() in drivers/infiniband/core/uverbs_cmd.c be updated to
account for it?

Currently, ib_uverbs_post_send() only checks for IB_QPT_UD to identify
unconnected QPs:

drivers/infiniband/core/uverbs_cmd.c:ib_uverbs_post_send() {
    ...
    is_ud = qp->qp_type == IB_QPT_UD;
    ...
}

If a send request is posted for an IB_QPT_RU QP via this legacy API, the core
will fall back to connected WR parsing, allocating struct ib_send_wr instead
of an unconnected WR struct with destination fields.

When this malformed WR is passed to an unconnected transport driver's
post_send op, the driver might attempt to access missing fields, which could
result in an out-of-bounds read or type confusion.

Should ib_uverbs_post_send() either parse IB_QPT_RU as an unconnected WR or
explicitly reject the operation with -EINVAL to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-uet-ib-core-v1-0-a0e626117113@broadcom.com?part=4