Re: [PATCH for-next v11 11/11] RDMA/rxe: Convert mca read locking to RCU

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Feb 08, 2022 at 03:16:45PM -0600, Bob Pearson wrote:
>  	/* check to see if the qp is already a member of the group */
> -	spin_lock_bh(&rxe->mcg_lock);
> -	list_for_each_entry(mca, &mcg->qp_list, qp_list) {
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(mca, &mcg->qp_list, qp_list) {
>  		if (mca->qp == qp) {

The use of mca->qp protected by RCU isn't Ok..

Look at the free path:

> +static void __rxe_cleanup_mca_rcu(struct rxe_mca *mca, struct rxe_mcg *mcg)
>  {
> +	list_del_rcu(&mca->qp_list);
>  
>  	atomic_dec(&mcg->qp_num);
>  	atomic_dec(&mcg->rxe->mcg_attach);
>  	atomic_dec(&mca->qp->mcg_num);
>  	rxe_drop_ref(mca->qp);
>  
> +	kfree_rcu(mca);

So the mca deref won't segfault but mca->qp is garbage now since it
might have been freed and reallocated due to the rxe_drop_ref()

It looks easy enough to fix, just use a call_rcu to free the thing
instead of kfree_rcu and do the rxe_drop_ref, and maybe others, inside
the call_rcu function.

> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> index 9b21cbb22602..c2cab85c6576 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> @@ -265,15 +265,15 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
>  	qp_array = kmalloc_array(nmax, sizeof(qp), GFP_KERNEL);
>  	n = 0;
>  
> -	spin_lock_bh(&rxe->mcg_lock);
> -	list_for_each_entry(mca, &mcg->qp_list, qp_list) {
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(mca, &mcg->qp_list, qp_list) {
>  		/* protect the qp pointers in the list */
>  		rxe_add_ref(mca->qp);

And this one could use after free qp

If the mca->qp is guarenteed to have a valid refcount as above then it
is OK.

Jason