Skip to content

Commit a44a3f1

Browse files
ohartoovrleon
ohartoov
authored and
rleon
committed
RDMA/uverbs: Fix mmap_lock/disassociation_lock circular dependency
Commit 51976c6 ("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: 51976c6 ("RDMA/core: Provide rdma_user_mmap_disassociate() to disassociate mmap pages") Signed-off-by: Or Har-Toov <ohartoov@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> Signed-off-by: Edward Srouji <edwards@nvidia.com> Link: https://patch.msgid.link/20260811-fix-mmap-lockdep-v1-1-1151b41063b4@nvidia.com Acked-by: Junxian Huang <huangjunxian6@hisilicon.com> Signed-off-by: Leon Romanovsky <leon@kernel.org>
1 parent ef9fbe1 commit a44a3f1

3 files changed

Lines changed: 11 additions & 17 deletions

File tree

‎drivers/infiniband/core/rdma_core.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ void ib_uverbs_release_file(struct kref *ref)
6969

7070
if (file->disassociate_page)
7171
__free_pages(file->disassociate_page, 0);
72-
mutex_destroy(&file->disassociation_lock);
7372
mutex_destroy(&file->umap_lock);
7473
mutex_destroy(&file->ucontext_lock);
7574
kfree(file);

‎drivers/infiniband/core/uverbs_main.c‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -644,12 +644,15 @@ static int ib_uverbs_mmap(struct file *filp, struct vm_area_struct *vma)
644644
goto out;
645645
}
646646

647-
mutex_lock(&file->disassociation_lock);
647+
if (!down_read_trylock(&file->hw_destroy_rwsem)) {
648+
ret = -EIO;
649+
goto out;
650+
}
648651

649652
vma->vm_ops = &rdma_umap_ops;
650653
ret = ucontext->device->ops.mmap(ucontext, vma);
651654

652-
mutex_unlock(&file->disassociation_lock);
655+
up_read(&file->hw_destroy_rwsem);
653656
out:
654657
srcu_read_unlock(&file->device->disassociate_srcu, srcu_key);
655658
return ret;
@@ -671,7 +674,6 @@ static void rdma_umap_open(struct vm_area_struct *vma)
671674
/* We are racing with disassociation */
672675
if (!down_read_trylock(&ufile->hw_destroy_rwsem))
673676
goto out_zap;
674-
mutex_lock(&ufile->disassociation_lock);
675677

676678
/*
677679
* Disassociation already completed, the VMA should already be zapped.
@@ -684,12 +686,10 @@ static void rdma_umap_open(struct vm_area_struct *vma)
684686
goto out_unlock;
685687
rdma_umap_priv_init(priv, vma, opriv->entry);
686688

687-
mutex_unlock(&ufile->disassociation_lock);
688689
up_read(&ufile->hw_destroy_rwsem);
689690
return;
690691

691692
out_unlock:
692-
mutex_unlock(&ufile->disassociation_lock);
693693
up_read(&ufile->hw_destroy_rwsem);
694694
out_zap:
695695
/*
@@ -773,7 +773,7 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
773773
{
774774
struct rdma_umap_priv *priv, *next_priv;
775775

776-
mutex_lock(&ufile->disassociation_lock);
776+
lockdep_assert_held_write(&ufile->hw_destroy_rwsem);
777777

778778
while (1) {
779779
struct mm_struct *mm = NULL;
@@ -799,10 +799,8 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
799799
break;
800800
}
801801
mutex_unlock(&ufile->umap_lock);
802-
if (!mm) {
803-
mutex_unlock(&ufile->disassociation_lock);
802+
if (!mm)
804803
return;
805-
}
806804

807805
/*
808806
* The umap_lock is nested under mmap_lock since it used within
@@ -832,8 +830,6 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
832830
mmap_read_unlock(mm);
833831
mmput(mm);
834832
}
835-
836-
mutex_unlock(&ufile->disassociation_lock);
837833
}
838834

839835
/**
@@ -851,8 +847,11 @@ void rdma_user_mmap_disassociate(struct ib_device *device)
851847

852848
mutex_lock(&uverbs_dev->lists_mutex);
853849
list_for_each_entry(ufile, &uverbs_dev->uverbs_file_list, list) {
854-
if (ufile->ucontext)
850+
if (ufile->ucontext) {
851+
down_write(&ufile->hw_destroy_rwsem);
855852
uverbs_user_mmap_disassociate(ufile);
853+
up_write(&ufile->hw_destroy_rwsem);
854+
}
856855
}
857856
mutex_unlock(&uverbs_dev->lists_mutex);
858857
}
@@ -927,8 +926,6 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)
927926
mutex_init(&file->umap_lock);
928927
INIT_LIST_HEAD(&file->umaps);
929928

930-
mutex_init(&file->disassociation_lock);
931-
932929
filp->private_data = file;
933930
list_add_tail(&file->list, &dev->uverbs_file_list);
934931
mutex_unlock(&dev->lists_mutex);

‎include/rdma/uverbs_types.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,6 @@ struct ib_uverbs_file {
180180
struct page *disassociate_page;
181181

182182
struct xarray idr;
183-
184-
struct mutex disassociation_lock;
185183
};
186184

187185
extern const struct uverbs_obj_type_class uverbs_idr_class;

0 commit comments

Comments
 (0)