Re: [PATCH rdma-next] RDMA/uverbs: Fix mmap_lock/disassociation_lock circular dependency

On 2026/8/12 0:19, Edward Srouji wrote:
> From: Or Har-Toov <[email protected]>
> 
> Commit 51976c6cd786 ("RDMA/core: Provide rdma_user_mmap_disassociate()
> to disassociate mmap pages") introduced disassociation_lock to protect
> new mmap registrations against uverbs_user_mmap_disassociate(), but
> created an ABBA deadlock:
> 
>   Thread A (mmap / fork):
>     mmap_lock -> disassociation_lock
> 
>   Thread B (disassociate):
>     disassociation_lock -> mmap_lock
> 
> Fix by removing disassociation_lock entirely and using the pre-existing
> hw_destroy_rwsem instead. hw_destroy_rwsem already provides the same
> protection: rdma_umap_open() and ib_uverbs_mmap() both use
> down_read_trylock() before registering a new VMA, so holding hw_destroy_rwsem
> in uverbs_user_mmap_disassociate() is sufficient to block new registrations.
> trylock is used in both mmap paths (not blocking down_read) because
> mmap_lock is already held on entry, and uverbs_user_mmap_disassociate()
> acquires mmap_lock internally — a blocking read would recreate the same
> deadlock.
> 
> The only caller that was not taking hw_destroy_rwsem for write was
> rdma_user_mmap_disassociate(). Fix it to take the rwsem per-ufile while
> iterating under lists_mutex.  This is safe because ib_uverbs_close()
> releases hw_destroy_rwsem entirely before acquiring lists_mutex, so the
> two locks are never held simultaneously.
> 
> lockdep warning:
> 
>  [  776.654252] ======================================================
>  [  776.655214] WARNING: possible circular locking dependency detected
>  [  776.656167] 6.18.0for-upstream_debug_94e244d9ccab #1 Not tainted
>  [  776.657114] ------------------------------------------------------
>  [  776.658087] devlink/14824 is trying to acquire lock:
>  [  776.658879] ffff88811170c800 (&mm->mmap_lock){++++}-{4:4}, at: uverbs_user_mmap_disassociate+0x168/0x780 [ib_uverbs]
>  [  776.660479]
>  [  776.660479] but task is already holding lock:
>  [  776.661460] ffff888142d92b08 (&file->disassociation_lock){+.+.}-{4:4}, at: uverbs_user_mmap_disassociate+0x39/0x780 [ib_uverbs]
>  [  776.663177]
>  [  776.663177] which lock already depends on the new lock.
>  [  776.663177]
>  [  776.664525]
>  [  776.664525] the existing dependency chain (in reverse order) is:
>  [  776.665724]
>  [  776.665724] -> #2 (&file->disassociation_lock){+.+.}-{4:4}:
>  [  776.666887]        __mutex_lock+0x16d/0x2330
>  [  776.667633]        rdma_umap_open+0x129/0x280 [ib_uverbs]
>  [  776.668489]        dup_mmap+0xa40/0x1790
>  [  776.669170]        copy_process+0x5dd2/0x6170
>  [  776.669933]        kernel_clone+0xb6/0x610
>  [  776.670636]        __do_sys_clone+0xb5/0xf0
>  [  776.671354]        do_syscall_64+0x70/0x12e0
>  [  776.672083]        entry_SYSCALL_64_after_hwframe+0x4b/0x53
>  [  776.672940]
>  [  776.672940] -> #1 (&mm->mmap_lock/1){+.+.}-{4:4}:
>  [  776.673985]        down_write_nested+0x90/0x1e0
>  [  776.674751]        dup_mmap+0x201/0x1790
>  [  776.675448]        copy_process+0x5dd2/0x6170
>  [  776.676180]        kernel_clone+0xb6/0x610
>  [  776.676904]        __do_sys_clone+0xb5/0xf0
>  [  776.677615]        do_syscall_64+0x70/0x12e0
>  [  776.678351]        entry_SYSCALL_64_after_hwframe+0x4b/0x53
>  [  776.679239]
>  [  776.679239] -> #0 (&mm->mmap_lock){++++}-{4:4}:
>  [  776.680253]        __lock_acquire+0x18c6/0x2ec0
>  [  776.681018]        lock_acquire+0x10e/0x2e0
>  [  776.681742]        down_read+0x95/0x430
>  [  776.682395]        uverbs_user_mmap_disassociate+0x168/0x780 [ib_uverbs]
>  [  776.683436]        uverbs_destroy_ufile_hw+0x1ae/0x270 [ib_uverbs]
>  [  776.684416]        ib_uverbs_remove_one+0x22b/0x420 [ib_uverbs]
>  [  776.685371]        remove_client_context+0xa6/0xf0 [ib_core]
>  [  776.686342]        disable_device+0x12b/0x240 [ib_core]
>  [  776.687249]        __ib_unregister_device+0x269/0x460 [ib_core]
>  [  776.688233]        ib_unregister_device+0x21/0x30 [ib_core]
>  [  776.689140]        mlx5r_remove+0xd0/0x170 [mlx5_ib]
>  [  776.689999]        device_release_driver_internal+0x3b2/0x560
>  [  776.694876]        bus_remove_device+0x1f5/0x3e0
>  [  776.695638]        device_del+0x3b9/0x990
>  [  776.696329]        mlx5_detach_device+0x17e/0x350 [mlx5_core]
>  [  776.697429]        mlx5_unload_one_devl_locked+0x3f/0xb0 [mlx5_core]
>  [  776.698578]        mlx5_devlink_reload_down+0x1f9/0x550 [mlx5_core]
>  [  776.699712]        devlink_reload+0x13e/0x680
>  [  776.700456]        devlink_nl_reload_doit+0xc29/0x1160
>  [  776.701293]        genl_family_rcv_msg_doit+0x1c9/0x2a0
>  [  776.702135]        genl_rcv_msg+0x3f0/0x6b0
>  [  776.702854]        netlink_rcv_skb+0x11d/0x370
>  [  776.703605]        genl_rcv+0x24/0x40
>  [  776.704236]        netlink_unicast+0x5b4/0x970
>  [  776.704984]        netlink_sendmsg+0x730/0xbf0
>  [  776.705748]        __sock_sendmsg+0xc5/0x190
>  [  776.706461]        __sys_sendto+0x201/0x2f0
>  [  776.707188]        __x64_sys_sendto+0xdc/0x1b0
>  [  776.707931]        do_syscall_64+0x70/0x12e0
>  [  776.708643]        entry_SYSCALL_64_after_hwframe+0x4b/0x53
>  [  776.709546]
>  [  776.709546] other info that might help us debug this:
>  [  776.709546]
>  [  776.710910] Chain exists of:
>  [  776.710910]   &mm->mmap_lock --> &mm->mmap_lock/1 --> &file->disassociation_lock
>  [  776.710910]
>  [  776.712805]  Possible unsafe locking scenario:
>  [  776.712805]
>  [  776.713828]        CPU0                    CPU1
>  [  776.714589]        ----                    ----
>  [  776.715347]   lock(&file->disassociation_lock);
>  [  776.716097]                                lock(&mm->mmap_lock/1);
>  [  776.717067]                                lock(&file->disassociation_lock);
>  [  776.718199]   rlock(&mm->mmap_lock);
>  [  776.718857]
>  [  776.718857]  *** DEADLOCK ***
> 
> Fixes: 51976c6cd786 ("RDMA/core: Provide rdma_user_mmap_disassociate() to disassociate mmap pages")
> Signed-off-by: Or Har-Toov <[email protected]>
> Signed-off-by: Leon Romanovsky <[email protected]>
> Signed-off-by: Edward Srouji <[email protected]>

Acked-by: Junxian Huang <[email protected]>

> ---
>  drivers/infiniband/core/rdma_core.c   |  1 -
>  drivers/infiniband/core/uverbs_main.c | 25 +++++++++++--------------
>  include/rdma/uverbs_types.h           |  2 --
>  3 files changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index fd5651c003aed3178188b4f89ede86d893676a9a..a7cbe643e33c6e5247a8f4eebc3b8ab1d9a66e4f 100644
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -69,7 +69,6 @@ void ib_uverbs_release_file(struct kref *ref)
>  
>  	if (file->disassociate_page)
>  		__free_pages(file->disassociate_page, 0);
> -	mutex_destroy(&file->disassociation_lock);
>  	mutex_destroy(&file->umap_lock);
>  	mutex_destroy(&file->ucontext_lock);
>  	kfree(file);
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 3ccf58e96aedebb6f8886ef5b8fad5660475c4c7..5be21fca4948272b9a66e8ee357ca9432556c887 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -643,12 +643,15 @@ static int ib_uverbs_mmap(struct file *filp, struct vm_area_struct *vma)
>  		goto out;
>  	}
>  
> -	mutex_lock(&file->disassociation_lock);
> +	if (!down_read_trylock(&file->hw_destroy_rwsem)) {
> +		ret = -EIO;
> +		goto out;
> +	}
>  
>  	vma->vm_ops = &rdma_umap_ops;
>  	ret = ucontext->device->ops.mmap(ucontext, vma);
>  
> -	mutex_unlock(&file->disassociation_lock);
> +	up_read(&file->hw_destroy_rwsem);
>  out:
>  	srcu_read_unlock(&file->device->disassociate_srcu, srcu_key);
>  	return ret;
> @@ -670,7 +673,6 @@ static void rdma_umap_open(struct vm_area_struct *vma)
>  	/* We are racing with disassociation */
>  	if (!down_read_trylock(&ufile->hw_destroy_rwsem))
>  		goto out_zap;
> -	mutex_lock(&ufile->disassociation_lock);
>  
>  	/*
>  	 * Disassociation already completed, the VMA should already be zapped.
> @@ -683,12 +685,10 @@ static void rdma_umap_open(struct vm_area_struct *vma)
>  		goto out_unlock;
>  	rdma_umap_priv_init(priv, vma, opriv->entry);
>  
> -	mutex_unlock(&ufile->disassociation_lock);
>  	up_read(&ufile->hw_destroy_rwsem);
>  	return;
>  
>  out_unlock:
> -	mutex_unlock(&ufile->disassociation_lock);
>  	up_read(&ufile->hw_destroy_rwsem);
>  out_zap:
>  	/*
> @@ -772,7 +772,7 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
>  {
>  	struct rdma_umap_priv *priv, *next_priv;
>  
> -	mutex_lock(&ufile->disassociation_lock);
> +	lockdep_assert_held_write(&ufile->hw_destroy_rwsem);
>  
>  	while (1) {
>  		struct mm_struct *mm = NULL;
> @@ -798,10 +798,8 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
>  			break;
>  		}
>  		mutex_unlock(&ufile->umap_lock);
> -		if (!mm) {
> -			mutex_unlock(&ufile->disassociation_lock);
> +		if (!mm)
>  			return;
> -		}
>  
>  		/*
>  		 * The umap_lock is nested under mmap_lock since it used within
> @@ -831,8 +829,6 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
>  		mmap_read_unlock(mm);
>  		mmput(mm);
>  	}
> -
> -	mutex_unlock(&ufile->disassociation_lock);
>  }
>  
>  /**
> @@ -850,8 +846,11 @@ void rdma_user_mmap_disassociate(struct ib_device *device)
>  
>  	mutex_lock(&uverbs_dev->lists_mutex);
>  	list_for_each_entry(ufile, &uverbs_dev->uverbs_file_list, list) {
> -		if (ufile->ucontext)
> +		if (ufile->ucontext) {
> +			down_write(&ufile->hw_destroy_rwsem);
>  			uverbs_user_mmap_disassociate(ufile);
> +			up_write(&ufile->hw_destroy_rwsem);
> +		}
>  	}
>  	mutex_unlock(&uverbs_dev->lists_mutex);
>  }
> @@ -926,8 +925,6 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)
>  	mutex_init(&file->umap_lock);
>  	INIT_LIST_HEAD(&file->umaps);
>  
> -	mutex_init(&file->disassociation_lock);
> -
>  	filp->private_data = file;
>  	list_add_tail(&file->list, &dev->uverbs_file_list);
>  	mutex_unlock(&dev->lists_mutex);
> diff --git a/include/rdma/uverbs_types.h b/include/rdma/uverbs_types.h
> index 5a07f9a6dcd1f6e2faaf2df25d2661d5bf2a170e..6f3622892c0cc6d71890f4ece7bd3a0c59432ee2 100644
> --- a/include/rdma/uverbs_types.h
> +++ b/include/rdma/uverbs_types.h
> @@ -180,8 +180,6 @@ struct ib_uverbs_file {
>  	struct page *disassociate_page;
>  
>  	struct xarray		idr;
> -
> -	struct mutex disassociation_lock;
>  };
>  
>  extern const struct uverbs_obj_type_class uverbs_idr_class;
> 
> ---
> base-commit: a12d9145145b21c50531afb6e3f711b1f34e1465
> change-id: 20260811-fix-mmap-lockdep-91da89a3f37c
> 
> Best regards,