[PATCH] uio: fix possible circular locking dependency

From: xiubli
Date: Mon Jul 30 2018 - 03:11:59 EST


From: Xiubo Li <xiubli@xxxxxxxxxx>

The call trace:
XXX/1910 is trying to acquire lock:
(&mm->mmap_sem){++++++}, at: [<ffffffff97008c87>] might_fault+0x57/0xb0

but task is already holding lock:
(&idev->info_lock){+.+...}, at: [<ffffffffc0638a06>] uio_write+0x46/0x130 [uio]

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&idev->info_lock){+.+...}:
[<ffffffff96f31fc9>] lock_acquire+0x99/0x1e0
[<ffffffff975edad3>] mutex_lock_nested+0x93/0x410
[<ffffffffc063873d>] uio_mmap+0x2d/0x170 [uio]
[<ffffffff97016b58>] mmap_region+0x428/0x650
[<ffffffff97017138>] do_mmap+0x3b8/0x4e0
[<ffffffff96ffaba3>] vm_mmap_pgoff+0xd3/0x120
[<ffffffff97015261>] SyS_mmap_pgoff+0x1f1/0x270
[<ffffffff96e387c2>] SyS_mmap+0x22/0x30
[<ffffffff975ff315>] system_call_fastpath+0x1c/0x21

-> #0 (&mm->mmap_sem){++++++}:
[<ffffffff96f30e9c>] __lock_acquire+0xdac/0x15f0
[<ffffffff96f31fc9>] lock_acquire+0x99/0x1e0
[<ffffffff97008cb4>] might_fault+0x84/0xb0
[<ffffffffc0638a74>] uio_write+0xb4/0x130 [uio]
[<ffffffff9706ffa3>] vfs_write+0xc3/0x1f0
[<ffffffff97070e2a>] SyS_write+0x8a/0x100
[<ffffffff975ff315>] system_call_fastpath+0x1c/0x21

other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&idev->info_lock);
lock(&mm->mmap_sem);
lock(&idev->info_lock);
lock(&mm->mmap_sem);

*** DEADLOCK ***
1 lock held by XXX/1910:
#0: (&idev->info_lock){+.+...}, at: [<ffffffffc0638a06>] uio_write+0x46/0x130 [uio]

stack backtrace:
CPU: 0 PID: 1910 Comm: XXX Kdump: loaded Not tainted #1
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
Call Trace:
[<ffffffff975e9211>] dump_stack+0x19/0x1b
[<ffffffff975e260a>] print_circular_bug+0x1f9/0x207
[<ffffffff96f2f6a7>] check_prevs_add+0x957/0x960
[<ffffffff96f30e9c>] __lock_acquire+0xdac/0x15f0
[<ffffffff96f2fb19>] ? mark_held_locks+0xb9/0x140
[<ffffffff96f31fc9>] lock_acquire+0x99/0x1e0
[<ffffffff97008c87>] ? might_fault+0x57/0xb0
[<ffffffff97008cb4>] might_fault+0x84/0xb0
[<ffffffff97008c87>] ? might_fault+0x57/0xb0
[<ffffffffc0638a74>] uio_write+0xb4/0x130 [uio]
[<ffffffff9706ffa3>] vfs_write+0xc3/0x1f0
[<ffffffff9709349c>] ? fget_light+0xfc/0x510
[<ffffffff97070e2a>] SyS_write+0x8a/0x100
[<ffffffff975ff315>] system_call_fastpath+0x1c/0x21

Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
drivers/uio/uio.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 5d421d7..7b5daba 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -625,6 +625,12 @@ static ssize_t uio_write(struct file *filep, const char __user *buf,
ssize_t retval;
s32 irq_on;

+ if (count != sizeof(s32))
+ return -EINVAL;
+
+ if (copy_from_user(&irq_on, buf, count))
+ return -EFAULT;
+
mutex_lock(&idev->info_lock);
if (!idev->info) {
retval = -EINVAL;
@@ -636,21 +642,11 @@ static ssize_t uio_write(struct file *filep, const char __user *buf,
goto out;
}

- if (count != sizeof(s32)) {
- retval = -EINVAL;
- goto out;
- }
-
if (!idev->info->irqcontrol) {
retval = -ENOSYS;
goto out;
}

- if (copy_from_user(&irq_on, buf, count)) {
- retval = -EFAULT;
- goto out;
- }
-
retval = idev->info->irqcontrol(idev->info, irq_on);

out:
--
1.8.3.1