On Tue, Jun 19, 2018 at 08:28:33AM +0300, Leon Romanovsky wrote:
> From: Guy Levi <guyle@xxxxxxxxxxxx>
>
> During uverbs' modify_qp, the port type is inferred using the port
> number in ib_uverbs_qp_dest struct (address vector). This port number is
> used to access that port's type info in the port_immutable array.
>
> According to IB spec (version 1.3) port number is not mentioned as a
> part of the AV which is used in modify QP. Using the port number field
> from the AV therefore leads to accesses to non-allocated memory when
> inferring the port type.
No, this isn't right at all..
Linux didn't quite follow the spec here, and it is confusing.
Spec wise, both of these ports exist for modify_qp, one is called the
'primary physical port' (pg 604) and the other is the 'alternate
path primary physical port' (pg 605):
Someone got confused when implementing this in Linux and wrongly
created *three* port numbes. The odd duck is IB_QP_PORT, which is only
used for some (probably wrong) security check. The ones that are used
are the two AV linked ones that sort of match the behavior expected
from the spec.
Confusing? Yes.
So, don't use the IB_QP_PORT value for anything.
The rdma_ah_attr requires a port number to be valid, and a few lines
below this:
> -static void copy_ah_attr_from_uverbs(struct ib_device *dev,
> +static void copy_ah_attr_from_uverbs(struct ib_qp *qp,
> struct rdma_ah_attr *rdma_attr,
> struct ib_uverbs_qp_dest *uverb_attr)
> {
> - rdma_attr->type = rdma_ah_find_type(dev, uverb_attr->port_num);
> + rdma_attr->type = rdma_ah_find_type(qp->device, qp->real_qp->port);
> if (uverb_attr->is_global) {
> rdma_ah_set_grh(rdma_attr, NULL,
> uverb_attr->flow_label,
We have this:
rdma_ah_set_port_num(rdma_attr, uverb_attr->port_num);
Which is nonsense to assert that it isn't required to be valid then go
and stuff it in the AH anyhow. No. It is required to valid and the
eventual consumer of the rdma_ah_attr does validate it (at least
it does now with Parav's series).
We have lots of other places touching the port number connected to the
attribute.
Thus, it is required to be provided by user space and required to be
valid.
This is just the same old bugs we have seen already: missing
validation of the port_num prior to using it in the kernel.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
