On Thu, Sep 17, 2020 at 07:19:50PM +0300, Leon Romanovsky wrote:
> 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.
Hm, OK
Jason
