> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -1210,6 +1210,11 @@ static int cma_req_handler(struct ib_cm_id *cm_id,
> struct ib_cm_event *ib_event)
> >        cm_id->context = conn_id;
> >        cm_id->cm_handler = cma_ib_handler;
> >
> > +       /*
> > +        * Protect against the user destroying conn_id from another
> thread
> > +        * until we're done accessing it.
> > +        */
> > +       atomic_inc(&conn_id->refcount);
> 
> This is a good catch, but I'm not sure I see why this is the right fix.
> 
> What prevents the destroy from happening right before the atomic_inc here?
> Does this just make the race window much smaller?

This is for a new request.  Until we call the user's event_handler (next line 
after bumping the refcount) with the new id, they can't destroy it.

> >                mutex_unlock(&conn_id->handler_mutex);
> > +               cma_deref_id(conn_id);
> >                rdma_destroy_id(&conn_id->id);
> 
> likewise this seems to drop the additional reference, and then use
> the conn_id.  Why can't it be destroyed right after the cma_deref_id
> leading to use-after-free?

That is a double free error by the user.  If they return a non-zero value from 
the callback, that indicates that the rdma_cm should destroy the id.  The user 
cannot use the id at that point.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to