Re: [PATCH rdma-next v1] RDMA/ucma: Fix use-after-free bug in ucma_create_uevent

[Jason Gunthorpe <jgg@nvidia.com>]

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