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 <[email protected]>
> > 
> > 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/[email protected]

> > Signed-off-by: Majd Dibbiny <[email protected]>
> > Signed-off-by: Or Gerlitz <[email protected]>
> > ---
> >  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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to