Hi Michal, > -----Original Message----- > From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma- > owner@xxxxxxxxxxxxxxx] On Behalf Of Michal Kalderon > Sent: Monday, June 11, 2018 2:20 AM > To: michal.kalderon@xxxxxxxxxx; Jason Gunthorpe <jgg@xxxxxxxxxxxx>; > dledford@xxxxxxxxxx > Cc: linux-rdma@xxxxxxxxxxxxxxx; yuval.bason@xxxxxxxxxx; Michal Kalderon > <Michal.Kalderon@xxxxxxxxxx>; Ariel Elior <Ariel.Elior@xxxxxxxxxx> > Subject: [PATCH for-rc] RDMA/qedr: Fix Null pointer dereference when running > over iWARP without RDMA-CM > > Some RoCE specific code in qedr_modify_qp was run over an iWARP device > when running perftest benchmarks without the -R option. > > The commit 3e44e0ee0893 ("IB/providers: Avoid null netdev check for RoCE") > exposed this. Dropping the check for NULL pointer on ndev in qedr_modify_qp > lead to a null pointer dereference when running over iWARP. Before the code > would identify ndev as being NULL and return an error. Previously get_gid_info_from_table() with valid gid index would return success and was doing things conditionally if ndev was set. > > Fixes: 3e44e0ee0893 ("IB/providers: Avoid null netdev check for RoCE") > Signed-off-by: Ariel Elior <Ariel.Elior@xxxxxxxxxx> > Signed-off-by: Michal Kalderon <Michal.Kalderon@xxxxxxxxxx> > --- > drivers/infiniband/hw/qedr/verbs.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/infiniband/hw/qedr/verbs.c > b/drivers/infiniband/hw/qedr/verbs.c > index 3f9afc0..f86223a 100644 > --- a/drivers/infiniband/hw/qedr/verbs.c > +++ b/drivers/infiniband/hw/qedr/verbs.c > @@ -1957,6 +1957,9 @@ int qedr_modify_qp(struct ib_qp *ibqp, struct > ib_qp_attr *attr, > } > > if (attr_mask & (IB_QP_AV | IB_QP_PATH_MTU)) { > + if (rdma_protocol_iwarp(&dev->ibdev, 1)) > + return -EINVAL; > + RB: parav@xxxxxxxxxxxx A check like rdma_protocol_roce() similar to what you have already for stack check makes it more clear that this is for roce. That likely preserves old behavior, I think. - if (attr_mask & (IB_QP_AV | IB_QP_PATH_MTU)) { + if (attr_mask & (IB_QP_AV | IB_QP_PATH_MTU) && + rdma_protocol_roce(&dev->ibdev, 1)) { Many checks in this function are for RoCE such as IB_QP_RETRY_CNT, IB_QP_RNR_RETRY, IB_QP_RQ_PSN, IB_QP_MIN_RNR_TIMER, IB_QP_SQ_PSN, IB_QP_QKEY, IB_QP_PKEY_INDEX. So for for-next, (not for-rc), 1. You should refactor the function to split code for roce and iwarp so that right checks and settings are done for right transport. It makes code more clear. 2. I would expect that ib_modify_qp_is_ok() should be extended for iWarp as well, so that when IB_QP_AV | IB_QP_PATH_MTU is set, provider driver doesn't have to do this check to return failure error code. 3. ib_modify_qp_is_ok() already has input parameter to pass link_layer. It should be extended, to honor do right state specific mask checks for right link layer. -- 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
