mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-09-18 22:09:30 +02:00
RDMA/uverbs: Fix mmap_lock/disassociation_lock circular dependency
Commit51976c6cd7("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:51976c6cd7("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>
This commit is contained in:
committed by
Leon Romanovsky
parent
ef9fbe1b93
commit
a44a3f175e
@@ -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);
|
||||
|
||||
@@ -644,12 +644,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;
|
||||
@@ -671,7 +674,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.
|
||||
@@ -684,12 +686,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:
|
||||
/*
|
||||
@@ -773,7 +773,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;
|
||||
@@ -799,10 +799,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
|
||||
@@ -832,8 +830,6 @@ void uverbs_user_mmap_disassociate(struct ib_uverbs_file *ufile)
|
||||
mmap_read_unlock(mm);
|
||||
mmput(mm);
|
||||
}
|
||||
|
||||
mutex_unlock(&ufile->disassociation_lock);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -851,8 +847,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);
|
||||
}
|
||||
@@ -927,8 +926,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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user