On 17/2/2015 3:59 PM, Yann Droneaud wrote: > Hi, > > Le lundi 16 février 2015 à 22:28 +0100, Yann Droneaud a écrit : >> Le jeudi 05 février 2015 à 13:53 +0200, Or Gerlitz a écrit : >>> From: Majd Dibbiny <majd@xxxxxxxxxxxx> >>> >>> In some error flows, ib_mad_unregister_agent is being invoked also in cases where >> ^^^^^^^^^^^^^^^^^^^^^^^ >> ib_unregister_mad_agent() >> >>> the ib_mad_register_agent call failed (resulting in an illegal pointer in the >> ^^^^^^^^^^^^^^^^^^^^^ >> ib_register_mad_agent() >> >>> agent field). This causes a kernel crash in the error flow. >>> >>> Fix this by calling ib_unregister_mad_agent only for cases where >>> ib_register_mad_agent succeeded. >>> >> The code was checking for struct ib_mad_agent *agent not being NULL, >> while ib_register_mad_agent() returns a ERR_PTR() in case of error >> ... bad :( >> > After reading Jason's comments [1], I don't understand the purpose of > this patch. How can an ERR_PTR() value be present in the arrays ? > > A quick review makes me think that mthca and mlx4 error paths lacks NULL > assignment after calling ib_unregister_mad_agent(). In other words, qib > might be correct while the others should be fixed. You are right. Seems like I missed things here. I'll fix it and send a new version soon. > > [1] http://mid.gmane.org/20150205174337.GB31711@xxxxxxxxxxxxxxxxxxxx > >>> Signed-off-by: Majd Dibbiny <majd@xxxxxxxxxxxx> >>> Signed-off-by: Or Gerlitz <ogerlitz@xxxxxxxxxxxx> >>> --- >>> drivers/infiniband/hw/mlx4/mad.c | 2 +- >>> drivers/infiniband/hw/mthca/mthca_mad.c | 2 +- >>> drivers/infiniband/hw/qib/qib_mad.c | 2 +- >>> 3 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c >>> index 82a7dd8..6be0d2c 100644 >>> --- a/drivers/infiniband/hw/mlx4/mad.c >>> +++ b/drivers/infiniband/hw/mlx4/mad.c >>> @@ -907,7 +907,7 @@ int mlx4_ib_mad_init(struct mlx4_ib_dev *dev) >>> err: >>> for (p = 0; p < dev->num_ports; ++p) >>> for (q = 0; q <= 1; ++q) >>> - if (dev->send_agent[p][q]) >>> + if (!IS_ERR(dev->send_agent[p][q])) >>> ib_unregister_mad_agent(dev->send_agent[p][q]); >>> >>> return ret; >>> diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c >>> index 8881fa3..5f1a7ce 100644 >>> --- a/drivers/infiniband/hw/mthca/mthca_mad.c >>> +++ b/drivers/infiniband/hw/mthca/mthca_mad.c >>> @@ -317,7 +317,7 @@ int mthca_create_agents(struct mthca_dev *dev) >>> err: >>> for (p = 0; p < dev->limits.num_ports; ++p) >>> for (q = 0; q <= 1; ++q) >>> - if (dev->send_agent[p][q]) >>> + if (!IS_ERR(dev->send_agent[p][q])) >>> ib_unregister_mad_agent(dev->send_agent[p][q]); >>> >>> return ret; >>> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c >>> index 636be11..6c7cc80 100644 >>> --- a/drivers/infiniband/hw/qib/qib_mad.c >>> +++ b/drivers/infiniband/hw/qib/qib_mad.c >>> @@ -2499,7 +2499,7 @@ int qib_create_agents(struct qib_ibdev *dev) >>> err: >>> for (p = 0; p < dd->num_pports; p++) { >>> ibp = &dd->pport[p].ibport_data; >>> - if (ibp->send_agent) { >>> + if (!IS_ERR(ibp->send_agent)) { >>> agent = ibp->send_agent; >>> ibp->send_agent = NULL; >>> ib_unregister_mad_agent(agent); > Regards. > -- 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
