On Wed, Jun 20, 2018 at 02:13:16AM -0600, Jack Morgenstein wrote:
> 1. The low level drivers (mlx4 and mlx5) use IB_QP_PORT in their
> modify_qp calls. When IB_QP_PORT is set in the attr structure,
> modify_qp sets the port field in its structure to the provided port
> number.
This I didn't notice.. What are they doing with it I wonder? Are they
checking it during AH usage? That might make sense.. Hmm.. Looks like
a bunch of places just assume the primary AV and the qp->port are the
same thing. That makes sense, lets just do that??
This is what I suggest, what do you think?
This patch would need to be split into two (one for stable and one for
-rc) due to it also fixing a bug in a patch already in -next.. Leon if
Jack likes it can you queue it for test?
>From d1c153040c922fb8d759bbb04abcc02b2eb289b3 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
Date: Wed, 20 Jun 2018 11:56:08 -0600
Subject: [PATCH] IB/uverbs: Validate the port_num in the AV's during modify_qp
The port numbers are not verified before placing them in the rdma_ah_attr,
which causes copy_ah_attr_from_uverbs to go past the end of an array.
In Linux we expect the primary AV port number to always match the port
number specified in the IB_QP_PORT, check for this in all the modify_qp
paths.
Fixes: 498ca3c82a7b ("IB/core: Avoid accessing non-allocated memory when inferring port type")
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
---
drivers/infiniband/core/uverbs_cmd.c | 26 ++++++++++++++++++++++++--
drivers/infiniband/core/verbs.c | 22 ++++++++++++++--------
2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 908ee8ab32972f..d34179d2aa6d4b 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2027,13 +2027,35 @@ static int modify_qp(struct ib_uverbs_file *file,
attr->alt_timeout = cmd->base.alt_timeout;
attr->rate_limit = cmd->rate_limit;
- if (cmd->base.attr_mask & IB_QP_AV)
+ if (cmd->base.attr_mask & IB_QP_AV) {
+ unsigned int primary_port = (cmd->base.attr_mask & IB_QP_PORT) ?
+ cmd->base.port_num :
+ qp->port;
+
+ /*
+ * The Linux ABI specifies the primary port number both
+ * loosely in the struct (as IBA suggests) and also in the
+ * AV. The primary AV port number must always agree with the
+ * primary port number set by IB_QP_PORT.
+ */
+ if (cmd->base.dest.port_num != primary_port) {
+ ret = -EINVAL;
+ goto release_qp;
+ }
copy_ah_attr_from_uverbs(qp->device, &attr->ah_attr,
&cmd->base.dest);
+ }
+
+ if (cmd->base.attr_mask & IB_QP_ALT_PATH) {
+ if (!rdma_is_port_valid(qp->device,
+ cmd->base.alt_dest.port_num)) {
+ ret = -EINVAL;
+ goto release_qp;
+ }
- if (cmd->base.attr_mask & IB_QP_ALT_PATH)
copy_ah_attr_from_uverbs(qp->device, &attr->alt_ah_attr,
&cmd->base.alt_dest);
+ }
ret = ib_modify_qp_with_udata(qp, attr,
modify_qp_mask(qp->qp_type,
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index b0ad739a7bd099..f1affc62d0f48a 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1578,15 +1578,10 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
const struct ib_gid_attr *old_sgid_attr_alt_av;
int ret;
- /*
- * Today the core code can only handle alternate paths and APM for IB
- * ban them in roce mode.
- */
- if (attr_mask & IB_QP_ALT_PATH &&
- !rdma_protocol_ib(qp->device, attr->alt_ah_attr.port_num))
- return -EINVAL;
-
if (attr_mask & IB_QP_AV) {
+ if (port != attr->ah_attr.port_num)
+ return -EINVAL;
+
ret = rdma_fill_sgid_attr(qp->device, &attr->ah_attr,
&old_sgid_attr_av);
if (ret)
@@ -1604,6 +1599,17 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
&old_sgid_attr_alt_av);
if (ret)
goto out_av;
+
+ /*
+ * Today the core code can only handle alternate paths and APM
+ * for IB. Ban them in roce mode.
+ */
+ if (!(rdma_protocol_ib(qp->device,
+ attr->alt_ah_attr.port_num) &&
+ rdma_protocol_ib(qp->device, port))) {
+ ret = EINVAL;
+ goto out;
+ }
}
/*
--
2.17.1
--
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
