On Mon, Jan 25, 2021 at 02:15:56PM +0200, Leon Romanovsky wrote:
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index e17ba841e204..7ce4d9dea826 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -352,7 +352,13 @@ struct ib_device *cma_get_ib_dev(struct cma_device *cma_dev)
>
> struct cma_multicast {
> struct rdma_id_private *id_priv;
> - struct ib_sa_multicast *sa_mc;
> + union {
> + struct ib_sa_multicast *sa_mc;
> + struct {
> + struct work_struct work;
> + struct rdma_cm_event event;
> + } iboe_join;
> + };
> struct list_head list;
> void *context;
> struct sockaddr_storage addr;
> @@ -1839,6 +1845,12 @@ static void destroy_mc(struct rdma_id_private *id_priv,
> cma_igmp_send(ndev, &mgid, false);
> dev_put(ndev);
> }
> +
> + if (cancel_work_sync(&mc->iboe_join.work))
> + /* Compensate for cma_iboe_join_work_handler that
> + * didn't run.
> + */
> + cma_id_put(mc->id_priv);
Just get rid of the cma_id_get in cma_iboe_join_multicast() and don't
have this if
> }
> kfree(mc);
> }
> @@ -2702,6 +2714,32 @@ static int cma_query_ib_route(struct rdma_id_private *id_priv,
> return (id_priv->query_id < 0) ? id_priv->query_id : 0;
> }
>
> +static void cma_iboe_join_work_handler(struct work_struct *work)
> +{
> + struct cma_multicast *mc =
> + container_of(work, struct cma_multicast, iboe_join.work);
> + struct rdma_cm_event *event = &mc->iboe_join.event;
> + struct rdma_id_private *id_priv = mc->id_priv;
> +
> + mutex_lock(&id_priv->handler_mutex);
> + if (READ_ONCE(id_priv->state) == RDMA_CM_DESTROYING ||
> + READ_ONCE(id_priv->state) == RDMA_CM_DEVICE_REMOVAL)
> + goto out_unlock;
> +
> + if (cma_cm_event_handler(id_priv, event)) {
> + cma_id_put(id_priv);
> + destroy_id_handler_unlock(id_priv);
This is a problem, destroy_id_handler_unlock eventually will call
destroy_mc() which will deadlock. The IB side has the same bug. Since
multicast isn't use in-kernel and ucma doesn't return anything but 0,
this is all dead code, lets delete it and just leave a WARN_ON(ret)
In the IB side too
> + goto out;
> + }
> +
> +out_unlock:
> + mutex_unlock(&id_priv->handler_mutex);
> + cma_id_put(id_priv);
and this put too
Jason
