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. [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. -- Yann Droneaud OPTEYA -- 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
