On Thu, Sep 17, 2020 at 09:06:36AM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:43PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > Update the code to have similar destroy pattern like other IB objects.
> >
> > This change create asymmetry to the rdma_id_private create flow to make
> > sure that memory is managed by restrack.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/core/cma.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index f9ff8b7f05e7..24e09416de4f 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -1821,7 +1821,6 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> > {
> > cma_cancel_operation(id_priv, state);
> >
> > - rdma_restrack_del(&id_priv->res);
> > if (id_priv->cma_dev) {
> > if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
> > if (id_priv->cm_id.ib)
> > @@ -1847,6 +1846,7 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> > rdma_put_gid_attr(id_priv->id.route.addr.dev_addr.sgid_attr);
> >
> > put_net(id_priv->id.route.addr.dev_addr.net);
> > + rdma_restrack_del(&id_priv->res);
> > kfree(id_priv);
> > }
>
> This is wrong, rdma_restrack_del() has to be called before
> ib_destroy_cm_id() because restrack reaches into the cm_id and touches
> that memory:
>
> case RDMA_RESTRACK_CM_ID:
> return container_of(res, struct rdma_id_private,
> res)->id.device;
>
> Which will now be freed after this change.
We access "id" which is struct rdma_cm_id, while ib_destroy_cm_id()
releases something different struct ib_cm_id cm_id.ib.
"id" is embedded into id_priv released later in _destroy_id() function.
Thanks
>
> Jason
